Re: [PATCH 08/14] hw/sd/sd: Allow for inverting polarities of presence and write-protect GPIOs

[Bernhard Beschow]

Am 9. Januar 2025 11:40:10 UTC schrieb "Philippe Mathieu-Daudé" 
<philmd@linaro.org>:
>Hi Bernhard,
>
>On 8/1/25 10:25, Bernhard Beschow wrote:
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>>   hw/sd/sd.c | 12 ++++++++----
>>   1 file changed, 8 insertions(+), 4 deletions(-)
>
>
>> @@ -876,8 +878,8 @@ static void sd_reset(DeviceState *dev)
>>       sd->cmd_line = true;
>>       sd->multi_blk_cnt = 0;
>>   -    qemu_set_irq(sd->readonly_cb, sd_get_readonly(sd));
>> -    qemu_set_irq(sd->inserted_cb, sd_get_inserted(sd));
>> +    qemu_set_irq(sd->readonly_cb, sd_get_readonly(sd) ^ 
>> sd->readonly_active_low);
>
>Please embed in sd_get_readonly(),
>
>> +    qemu_set_irq(sd->inserted_cb, sd_get_inserted(sd) ^ 
>> sd->inserted_active_low);
>
>and sd_get_inserted().

Are you sure? I deliberately implemented it as is because embedding would 
change the internal logic of the device as well as SDCardClass::{get_inserted, 
get_readonly}.

Best regards,
Bernhard

>
>>   }
>>     static void sd_cardchange(void *opaque, bool load, Error **errp)
>> @@ -896,9 +898,9 @@ static void sd_cardchange(void *opaque, bool load, Error 
>> **errp)
>>       }
>>         if (sd->me_no_qdev_me_kill_mammoth_with_rocks) {
>> -        qemu_set_irq(sd->inserted_cb, inserted);
>> +        qemu_set_irq(sd->inserted_cb, inserted ^ sd->inserted_active_low);
>
>Use sd_get_inserted(),
>
>>           if (inserted) {
>> -            qemu_set_irq(sd->readonly_cb, readonly);
>> +            qemu_set_irq(sd->readonly_cb, readonly ^ 
>> sd->readonly_active_low);
>
>and sd_get_readonly() here.
>
>>           }
>>       } else {
>>           sdbus = SD_BUS(qdev_get_parent_bus(dev));
>> @@ -2797,6 +2799,8 @@ static void emmc_realize(DeviceState *dev, Error 
>> **errp)
>>     static const Property sdmmc_common_properties[] = {
>>       DEFINE_PROP_DRIVE("drive", SDState, blk),
>> +    DEFINE_PROP_BOOL("cd-active-low", SDState, inserted_active_low, false),
>> +    DEFINE_PROP_BOOL("wp-active-low", SDState, readonly_active_low, false),
>>   };
>With the requested changes:
>Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>