Re: [PATCH master] mci: dw_mmc: make reset control optional again

[Philipp Zabel <p.zabel@pengutronix.de>]

On Tue, 2021-11-02 at 09:06 +0100, Sascha Hauer wrote:
> On Mon, Nov 01, 2021 at 06:52:07PM +0100, Ahmad Fatoum wrote:
> > As documented in 90bdf1e5d1e4 ("mci: dw_mmc: match against StarFive MMC
> > compatibles"), it was intended for the reset to remain optional as to
> > not break existing users. Unfortunately, my later a3cf324593ea
> > ("mci: dw_mmc: add optional reset line") didn't heed that and made it
> > required, breaking SoCFPGA DW-MMC use as a result.
> > 
> > Revert that line to fix the regression.
> > 
> > Fixes: a3cf324593ea ("mci: dw_mmc: add optional reset line")
> > Reported-by: Ian Abbott <abbotti@mev.co.uk>
> > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > ---
> >  drivers/mci/dw_mmc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/mci/dw_mmc.c b/drivers/mci/dw_mmc.c
> > index b402090ab3cb..86c4f43e88f5 100644
> > --- a/drivers/mci/dw_mmc.c
> > +++ b/drivers/mci/dw_mmc.c
> > @@ -572,7 +572,7 @@ static int dw_mmc_probe(struct device_d *dev)
> >  
> > 
> >  	rst = reset_control_get(dev, "reset");
> 
> Philipp, the reset binding lists the reset-names property as optional.
> What's the expected behaviour of the reset_control_get() above when the
> reset-names property is not present in the device tree? Should it return
> an error or should it return the unnamed reset control?

I'd expect it to return -ENOENT.

The common reset binding only has reset-names optional for the single-
reset case - but that should be requested with reset_control_get(dev,
NULL) by the driver.
If the device specific binding specifies a reset-names propertiy, it
should either be required, or the resets property should be optional as
well, with either both or none present in the device tree.

In the kernel there's also reset_control_get_optional_... variants that
return NULL instead of -ENOENT to simplify the optional reset case.

I think the issue here may be that barebox of_reset_control_get() is
missing 3d81216fde46 ("reset: Fix of_reset_control_get() for consistent
return values") [1]. The driver should then ignore the -ENOENT error.

[1] https://lore.kernel.org/lkml/1441121311-31628-1-git-send-email-albeu@free.fr/

regards
Philipp

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox