mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Philipp Zabel <p.zabel@pengutronix.de>
To: Sascha Hauer <sha@pengutronix.de>, Philipp Zabel <pza@pengutronix.de>
Cc: Ahmad Fatoum <a.fatoum@pengutronix.de>,
	barebox@lists.infradead.org, Ian Abbott <abbotti@mev.co.uk>
Subject: Re: [PATCH master] mci: dw_mmc: make reset control optional again
Date: Tue, 02 Nov 2021 10:04:48 +0100	[thread overview]
Message-ID: <0ede56d957c0bb05ac4aa16e80669e406a9e6ac6.camel@pengutronix.de> (raw)
In-Reply-To: <20211102080651.GY25698@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

  reply	other threads:[~2021-11-02  9:06 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-01 17:52 Ahmad Fatoum
2021-11-02  8:06 ` Sascha Hauer
2021-11-02  9:04   ` Philipp Zabel [this message]
2021-11-08 10:28   ` Ahmad Fatoum
2021-11-10  8:16     ` Sascha Hauer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0ede56d957c0bb05ac4aa16e80669e406a9e6ac6.camel@pengutronix.de \
    --to=p.zabel@pengutronix.de \
    --cc=a.fatoum@pengutronix.de \
    --cc=abbotti@mev.co.uk \
    --cc=barebox@lists.infradead.org \
    --cc=pza@pengutronix.de \
    --cc=sha@pengutronix.de \
    --subject='Re: [PATCH master] mci: dw_mmc: make reset control optional again' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox