diff mbox series

[v4,4/4] net: ethernet: ti: davinci_mdio: Deferring probe when soc_device_match() returns NULL

Message ID 20221108181144.433087-5-nfrayer@baylibre.com
State New
Headers
Series soc: ti: Add module build support to the socinfo |

Commit Message

Nicolas Frayer Nov. 8, 2022, 6:11 p.m. UTC 
  When the k3 socinfo driver is built as a module, there is a possibility
that it will probe after the davinci mdio driver. By deferring the mdio
probe we allow the k3 socinfo to probe and register the
soc_device_attribute structure needed by the mdio driver.

Signed-off-by: Nicolas Frayer <nfrayer@baylibre.com>
---
 drivers/net/ethernet/ti/davinci_mdio.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Paolo Abeni Nov. 10, 2022, 11:21 a.m. UTC | #1 
Hello,

On Tue, 2022-11-08 at 19:11 +0100, Nicolas Frayer wrote:
> When the k3 socinfo driver is built as a module, there is a possibility
> that it will probe after the davinci mdio driver. By deferring the mdio
> probe we allow the k3 socinfo to probe and register the
> soc_device_attribute structure needed by the mdio driver.
> 
> Signed-off-by: Nicolas Frayer <nfrayer@baylibre.com>

I guess this one will go via the arm tree, right?

I'm dropping it from the netdev PW.

Thanks,

Paolo
Nicolas Frayer Nov. 16, 2022, 10:52 a.m. UTC | #2 
Le jeu. 10 nov. 2022 à 12:21, Paolo Abeni <pabeni@redhat.com> a écrit :
>
> Hello,
>
> On Tue, 2022-11-08 at 19:11 +0100, Nicolas Frayer wrote:
> > When the k3 socinfo driver is built as a module, there is a possibility
> > that it will probe after the davinci mdio driver. By deferring the mdio
> > probe we allow the k3 socinfo to probe and register the
> > soc_device_attribute structure needed by the mdio driver.
> >
> > Signed-off-by: Nicolas Frayer <nfrayer@baylibre.com>
>
> I guess this one will go via the arm tree, right?
>
> I'm dropping it from the netdev PW.
>
> Thanks,
>
> Paolo
>
Hello Paolo,

I will resend this series as individual patches, so can you please
apply this mdio patch individually as it's independent from the others ?

Thanks,

Nicolas
Vignesh Raghavendra Nov. 23, 2022, 3:59 p.m. UTC | #3 
Hi Nicolas,

On 08/11/22 11:41 pm, Nicolas Frayer wrote:
> When the k3 socinfo driver is built as a module, there is a possibility
> that it will probe after the davinci mdio driver. By deferring the mdio
> probe we allow the k3 socinfo to probe and register the
> soc_device_attribute structure needed by the mdio driver.
> 
> Signed-off-by: Nicolas Frayer <nfrayer@baylibre.com>
> ---
>  drivers/net/ethernet/ti/davinci_mdio.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
> index 946b9753ccfb..095198b6b7be 100644
> --- a/drivers/net/ethernet/ti/davinci_mdio.c
> +++ b/drivers/net/ethernet/ti/davinci_mdio.c
> @@ -533,6 +533,10 @@ static int davinci_mdio_probe(struct platform_device *pdev)
>  		const struct soc_device_attribute *soc_match_data;
>  
>  		soc_match_data = soc_device_match(k3_mdio_socinfo);
> +
> +		if (!soc_match_data)
> +			return -EPROBE_DEFER;

I dont think this is right way to detect if socinfo driver is probed.
Per documentation of soc_device_match() , function will return NULL if
it does not match any of the entries in k3_mdio_socinfo (ie if we are
running on any platforms other that ones in the list)

Note that this driver is used on TI's 32 bit SoCs too that dont even
have a k3-socinfo driver equivalent. In such case, this code will end up
probe deferring indefinitely.

> +
>  		if (soc_match_data && soc_match_data->data) {
>  			const struct k3_mdio_soc_data *socdata =
>  						soc_match_data->data;
Kevin Hilman Nov. 23, 2022, 5:33 p.m. UTC | #4 
Hi Vignesh,

Vignesh Raghavendra <vigneshr@ti.com> writes:

> Hi Nicolas,
>
> On 08/11/22 11:41 pm, Nicolas Frayer wrote:
>> When the k3 socinfo driver is built as a module, there is a possibility
>> that it will probe after the davinci mdio driver. By deferring the mdio
>> probe we allow the k3 socinfo to probe and register the
>> soc_device_attribute structure needed by the mdio driver.
>> 
>> Signed-off-by: Nicolas Frayer <nfrayer@baylibre.com>
>> ---
>>  drivers/net/ethernet/ti/davinci_mdio.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>> 
>> diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
>> index 946b9753ccfb..095198b6b7be 100644
>> --- a/drivers/net/ethernet/ti/davinci_mdio.c
>> +++ b/drivers/net/ethernet/ti/davinci_mdio.c
>> @@ -533,6 +533,10 @@ static int davinci_mdio_probe(struct platform_device *pdev)
>>  		const struct soc_device_attribute *soc_match_data;
>>  
>>  		soc_match_data = soc_device_match(k3_mdio_socinfo);
>> +
>> +		if (!soc_match_data)
>> +			return -EPROBE_DEFER;
>
> I dont think this is right way to detect if socinfo driver is probed.
> Per documentation of soc_device_match() , function will return NULL if
> it does not match any of the entries in k3_mdio_socinfo (ie if we are
> running on any platforms other that ones in the list)
>
> Note that this driver is used on TI's 32 bit SoCs too that dont even
> have a k3-socinfo driver equivalent. In such case, this code will end up
> probe deferring indefinitely.

Yes, you're right.  This is not the right solution and this patch should
be dropped. We'll need to have a deeper look at socinfo to figure out
if/how it could be configured to support a fully modular kernel.

Kevin
diff mbox series

Patch

diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
index 946b9753ccfb..095198b6b7be 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -533,6 +533,10 @@  static int davinci_mdio_probe(struct platform_device *pdev)
 		const struct soc_device_attribute *soc_match_data;
 
 		soc_match_data = soc_device_match(k3_mdio_socinfo);
+
+		if (!soc_match_data)
+			return -EPROBE_DEFER;
+
 		if (soc_match_data && soc_match_data->data) {
 			const struct k3_mdio_soc_data *socdata =
 						soc_match_data->data;