Re: [PATCH net-next v4 11/12] microchip: lan865x: add driver support for Microchip's LAN865X MAC-PHY

Parthiban.Veerasooran
2024-04-29T09:47:57Z

Hi All,

On 28/04/24 1:27 am, Conor Dooley wrote:
> On Sat, Apr 27, 2024 at 09:19:34PM +0200, Ramón Nordin Rodriguez wrote:
>> Hi,
>>
>> For me the mac driver fails to probe with the following log
>> [ 0.123325] SPI driver lan865x has no spi_device_id for microchip,lan8651
>>
>> With this change the driver probes
>>
>> diff --git a/drivers/net/ethernet/microchip/lan865x/lan865x.c b/drivers/net/ethernet/microchip/lan865x/lan865x.c
>> index 9abefa8b9d9f..72a663f14f50 100644
>> --- a/drivers/net/ethernet/microchip/lan865x/lan865x.c
>> +++ b/drivers/net/ethernet/microchip/lan865x/lan865x.c
>> @@ -364,7 +364,7 @@ static void lan865x_remove(struct spi_device *spi)
>> }
>>
>> static const struct of_device_id lan865x_dt_ids[] = {
>> - { .compatible = "microchip,lan8651", "microchip,lan8650" },
> Huh, that's very strange. I don't see a single instance in the tree of a
> of_device_id struct like this with two compatibles like this (at least
> with a search of `rg "\.compatible.*\", \"" drivers/`.
>
> Given the fallbacks in the binding, only "microchip,lan8650" actually
> needs to be here.
>
>> + { .compatible = "microchip,lan865x", "microchip,lan8650" },
>> { /* Sentinel */ }
>> };
>> MODULE_DEVICE_TABLE(of, lan865x_dt_ids);
>>
>> Along with compatible = "microchip,lan865x" in the dts
> Just to be clear, the compatible w/ an x is unacceptable due to the
> wildcard and the binding should stay as-is. Whatever probing bugs
> the code has need to be resolved instead 🙂
>
Looks like, the below changes needed to work correctly,

lan865x.c:
- compatible string to be changed like below as it is a fallback for all
variants,
.compatible = "microchip,lan8650"
- DRV_NAME to be changed like below,
#define DRV_NAME "lan8650"

microchip,lan865x.example.dts for lan8650:
- compatible string to be changed like below,
.compatible = "microchip,lan8650";
OR
microchip,lan865x.example.dts for lan8651:
- compatible string to be changed like below,
compatible = "microchip,lan8651", "microchip,lan8650";

I tested with the above changes and there was no issues observed. Any
comments on this? Otherwise we can stick with these changes for the next
version.

Best regards,
Parthiban V

> Thanks,
> Conor.