LinuxPPC-Dev Archive on lore.kernel.org
 help / color / Atom feed
From: Dominique MARTINET <dominique.martinet@atmark-techno.com>
To: "Alice Guo (OSS)" <alice.guo@oss.nxp.com>
Cc: ulf.hansson@linaro.org, aymen.sghaier@nxp.com,
	geert+renesas@glider.be, rafael@kernel.org, airlied@linux.ie,
	mturquette@baylibre.com, dri-devel@lists.freedesktop.org,
	linux-kernel@vger.kernel.org, a.hajda@samsung.com,
	netdev@vger.kernel.org, linux-phy@lists.infradead.org,
	will@kernel.org, linux-clk@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org, wim@linux-watchdog.org,
	herbert@gondor.apana.org.au, horia.geanta@nxp.com,
	khilman@baylibre.com, joro@8bytes.org, narmstrong@baylibre.com,
	linux-staging@lists.linux.dev, iommu@lists.linux-foundation.org,
	peter.ujfalusi@gmail.com, kishon@ti.com, tony@atomide.com,
	linux-omap@vger.kernel.org, stern@rowland.harvard.edu,
	kuba@kernel.org, linus.walleij@linaro.org, linux@roeck-us.net,
	linux-media@vger.kernel.org, linux-watchdog@vger.kernel.org,
	Roy.Pledge@nxp.com, linux-pm@vger.kernel.org,
	linuxppc-dev@lists.ozlabs.org, edubezval@gmail.com,
	linux-gpio@vger.kernel.org, linux-mediatek@lists.infradead.org,
	ssantosh@kernel.org, matthias.bgg@gmail.com,
	linux-amlogic@lists.infradead.org, mchehab@kernel.org,
	linux-arm-kernel@lists.infradead.org, balbi@kernel.org,
	tomba@kernel.org, sboyd@kernel.org, gregkh@linuxfoundation.org,
	linux-usb@vger.kernel.org, linux-mmc@vger.kernel.org,
	adrian.hunter@intel.com, robert.foss@linaro.org,
	leoyang.li@nxp.com, linux@prisktech.co.nz, vkoul@kernel.org,
	linux-crypto@vger.kernel.org, daniel@ffwll.ch, j-keerthy@ti.com,
	dmaengine@vger.kernel.org, jyri.sarha@iki.fi,
	davem@davemloft.net
Subject: Re: [RFC v1 PATCH 3/3] driver: update all the code that use soc_device_match
Date: Mon, 19 Apr 2021 14:02:47 +0900
Message-ID: <YH0O907dfGY9jQRZ@atmark-techno.com> (raw)
In-Reply-To: <20210419042722.27554-4-alice.guo@oss.nxp.com>

Alice Guo (OSS) wrote on Mon, Apr 19, 2021 at 12:27:22PM +0800:
> From: Alice Guo <alice.guo@nxp.com>
> 
> Update all the code that use soc_device_match

A single patch might be difficult to accept for all components, a each
maintainer will probably want to have a say on their subsystem?

I would suggest to split these for a non-RFC version; a this will really
need to be case-by-case handling.

> because add support for soc_device_match returning -EPROBE_DEFER.

(English does not parse here for me)

I've only commented a couple of places in the code itself, but this
doesn't seem to add much support for errors, just sweep the problem
under the rug.

> Signed-off-by: Alice Guo <alice.guo@nxp.com>
> ---
> 
> diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
> index 5fae60f8c135..00c59aa217c1 100644
> --- a/drivers/bus/ti-sysc.c
> +++ b/drivers/bus/ti-sysc.c
> @@ -2909,7 +2909,7 @@ static int sysc_init_soc(struct sysc *ddata)
>  	}
>  
>  	match = soc_device_match(sysc_soc_feat_match);
> -	if (!match)
> +	if (!match || IS_ERR(match))
>  		return 0;

This function handles errors, I would recommend returning the error as
is if soc_device_match returned one so the probe can be retried later.

>  
>  	if (match->data)
> diff --git a/drivers/clk/renesas/r8a7795-cpg-mssr.c b/drivers/clk/renesas/r8a7795-cpg-mssr.c
> index c32d2c678046..90a18336a4c3 100644
> --- a/drivers/clk/renesas/r8a7795-cpg-mssr.c
> +++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c
> @@ -439,6 +439,7 @@ static const unsigned int r8a7795es2_mod_nullify[] __initconst = {
>  
>  static int __init r8a7795_cpg_mssr_init(struct device *dev)
>  {
> +	const struct soc_device_attribute *match;
>  	const struct rcar_gen3_cpg_pll_config *cpg_pll_config;
>  	u32 cpg_mode;
>  	int error;
> @@ -453,7 +454,8 @@ static int __init r8a7795_cpg_mssr_init(struct device *dev)
>  		return -EINVAL;
>  	}
>  
> -	if (soc_device_match(r8a7795es1)) {
> +	match = soc_device_match(r8a7795es1);
> +	if (!IS_ERR(match) && match) {

Same, return the error.
Assuming an error means no match will just lead to hard to debug
problems because the driver potentially assumed the wrong device when
it's just not ready yet.

>  		cpg_core_nullify_range(r8a7795_core_clks,
>  				       ARRAY_SIZE(r8a7795_core_clks),
>  				       R8A7795_CLK_S0D2, R8A7795_CLK_S0D12);
> [...]
> diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
> index eaaec0a55cc6..13a06b613379 100644
> --- a/drivers/iommu/ipmmu-vmsa.c
> +++ b/drivers/iommu/ipmmu-vmsa.c
> @@ -757,17 +757,20 @@ static const char * const devices_allowlist[] = {
>  
>  static bool ipmmu_device_is_allowed(struct device *dev)
>  {
> +	const struct soc_device_attribute *match1, *match2;
>  	unsigned int i;
>  
>  	/*
>  	 * R-Car Gen3 and RZ/G2 use the allow list to opt-in devices.
>  	 * For Other SoCs, this returns true anyway.
>  	 */
> -	if (!soc_device_match(soc_needs_opt_in))
> +	match1 = soc_device_match(soc_needs_opt_in);
> +	if (!IS_ERR(match1) && !match1)

I'm not sure what you intended to do, but !match1 already means there is
no error so the original code is identical.

In this case ipmmu_device_is_allowed does not allow errors so this is
one of the "difficult" drivers that require slightly more thinking.
It is only called in ipmmu_of_xlate which does return errors properly,
so in this case the most straightforward approach would be to make
ipmmu_device_is_allowed return an int and forward errors as well.



...
This is going to need quite some more work to be acceptable, in my
opinion, but I think it should be possible.

Thanks,
-- 
Dominique

  parent reply index

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-19  4:27 [RFC v1 PATCH 0/3] support soc_device_match to return -EPROBE_DEFER Alice Guo (OSS)
2021-04-19  4:27 ` [RFC v1 PATCH 1/3] drivers: soc: add support for soc_device_match returning -EPROBE_DEFER Alice Guo (OSS)
2021-04-19  4:49   ` Dominique MARTINET
2021-04-19  6:40     ` Alice Guo (OSS)
2021-04-19  8:20   ` Geert Uytterhoeven
2021-04-20 11:21     ` Dan Carpenter
2021-04-19  4:27 ` [RFC v1 PATCH 2/3] caam: add defer probe when the caam driver cannot identify SoC Alice Guo (OSS)
2021-04-19  4:27 ` [RFC v1 PATCH 3/3] driver: update all the code that use soc_device_match Alice Guo (OSS)
2021-04-19  5:02   ` Leon Romanovsky
2021-04-19  6:46     ` Alice Guo (OSS)
2021-04-19  5:02   ` Dominique MARTINET [this message]
2021-04-19  7:09     ` Alice Guo (OSS)
2021-04-19  9:03     ` Geert Uytterhoeven
2021-04-19  9:33       ` Dominique MARTINET
2021-04-19 12:16         ` Arnd Bergmann
2021-04-19 23:42           ` Dominique MARTINET
2021-04-20  9:07             ` Arnd Bergmann
2021-04-20  9:10             ` Arnd Bergmann
2021-04-19 13:36   ` Guenter Roeck
2021-04-20  9:40   ` Péter Ujfalusi

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=YH0O907dfGY9jQRZ@atmark-techno.com \
    --to=dominique.martinet@atmark-techno.com \
    --cc=Roy.Pledge@nxp.com \
    --cc=a.hajda@samsung.com \
    --cc=adrian.hunter@intel.com \
    --cc=airlied@linux.ie \
    --cc=alice.guo@oss.nxp.com \
    --cc=aymen.sghaier@nxp.com \
    --cc=balbi@kernel.org \
    --cc=daniel@ffwll.ch \
    --cc=davem@davemloft.net \
    --cc=dmaengine@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=edubezval@gmail.com \
    --cc=geert+renesas@glider.be \
    --cc=gregkh@linuxfoundation.org \
    --cc=herbert@gondor.apana.org.au \
    --cc=horia.geanta@nxp.com \
    --cc=iommu@lists.linux-foundation.org \
    --cc=j-keerthy@ti.com \
    --cc=joro@8bytes.org \
    --cc=jyri.sarha@iki.fi \
    --cc=khilman@baylibre.com \
    --cc=kishon@ti.com \
    --cc=kuba@kernel.org \
    --cc=leoyang.li@nxp.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-amlogic@lists.infradead.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=linux-mmc@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux-phy@lists.infradead.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=linux-usb@vger.kernel.org \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=linux@prisktech.co.nz \
    --cc=linux@roeck-us.net \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=matthias.bgg@gmail.com \
    --cc=mchehab@kernel.org \
    --cc=mturquette@baylibre.com \
    --cc=narmstrong@baylibre.com \
    --cc=netdev@vger.kernel.org \
    --cc=peter.ujfalusi@gmail.com \
    --cc=rafael@kernel.org \
    --cc=robert.foss@linaro.org \
    --cc=sboyd@kernel.org \
    --cc=ssantosh@kernel.org \
    --cc=stern@rowland.harvard.edu \
    --cc=tomba@kernel.org \
    --cc=tony@atomide.com \
    --cc=ulf.hansson@linaro.org \
    --cc=vkoul@kernel.org \
    --cc=will@kernel.org \
    --cc=wim@linux-watchdog.org \
    /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

LinuxPPC-Dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linuxppc-dev/0 linuxppc-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linuxppc-dev linuxppc-dev/ https://lore.kernel.org/linuxppc-dev \
		linuxppc-dev@lists.ozlabs.org linuxppc-dev@ozlabs.org
	public-inbox-index linuxppc-dev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.ozlabs.lists.linuxppc-dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git