Re: [PATCH v2 06/13] hw/sd/sd: Remove legacy sd_set_cb() in favor of GPIOs

[Peter Maydell]

On Mon, 27 Jan 2025 at 23:11, Bernhard Beschow <shentey@gmail.com> wrote:
>
>
>
> Am 27. Januar 2025 13:24:46 UTC schrieb Peter Maydell 
> <peter.maydell@linaro.org>:
> >On Sat, 11 Jan 2025 at 18:37, Bernhard Beschow <shentey@gmail.com> wrote:
> >>
> >> Commit ce5dd27534b0 "hw/sd: Remove omap2_mmc device" removed the last user 
> >> of
> >> sd_set_cb(). Rework this functionality into GPIOs.
> >>
> >> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> >> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> >
> >What is this for? We have a non-legacy API for "the SD controller
> >needs to know when the SD card is inserted or the readonly
> >status changes", which is that the controller implements the
> >SDBasClass set_inserted and set_readonly methods. (See the pl011
> >for an example.)
> >
> >I would prefer it if we used that consistently, rather than having
> >two mechanisms, one using GPIO lines and one using class methods.
> >I think we should delete the sd_set_cb() API and handling code
> >entirely.
>
> According to the Linux MMC controller DT schema, there are actually two ways 
> to implement cd and wp lines [1]. When implementing the imx8mp-evk board, I 
> thought I would need to model the GPIO style [2], hence I implemented it plus 
> the active low part on the SD card. Later I noticed that the card gets 
> detected anyway without the GPIO wiring, so I'm fine if the code gets removed 
> instead.

Even if you did need to implement that GPIO wiring, I think the
right way to do it is for the SD controller to implement a
subclass of SDBusClass so it can have its own implementations
of the set_inserted and set_readonly methods, and then it
is the SD controller object that has the GPIO lines, not
the SD card itself.

(I have a series almost ready to send out which QOMifies
the omap_mmc device and then deletes the "legacy" SD API,
including sd_set_cb().)

thanks
-- PMM