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

[Parthiban.Veerasooran]

Hi Andrew,

On 29/04/24 5:39 pm, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
>> 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.
> 
> As Conor said, this is probably relying on the fallback
> mechanism. Please look at other SPI devices, e.g. hwmon, and see how
> they probe for multiple different devices.
I just referred the below drivers for the spi devices handling along 
with the compatible,

https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/davicom/dm9051.c#L1239

https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/adi/adin1110.c#L1644

lan8650 - MAC-PHY chip
lan8651 - ETH Click-Mikroe with MAC-PHY chip

So, they are different in interface but not in functionality. There is 
no difference in the configuration. So let's consider lan8650 is the 
fallback option for lan8651.

By referring the above links, I have restructured the code like below to 
support with lan8651 fallback. Still there is no change in the existing 
device tree binding. This is the only change in lan865x.c.

static const struct spi_device_id spidev_spi_ids[] = {
         { .name = "lan8650" },
         {},
};

static const struct of_device_id lan865x_dt_ids[] = {
         { .compatible = "microchip,lan8650" },
         { /* Sentinel */ }
};
MODULE_DEVICE_TABLE(of, lan865x_dt_ids);

static struct spi_driver lan865x_driver = {
         .driver = {
                 .name = DRV_NAME,
                 .of_match_table = lan865x_dt_ids,
          },
         .probe = lan865x_probe,
         .remove = lan865x_remove,
         .id_table = spidev_spi_ids,
};

I also referred the below two links for the device tree binding and 
driver in case of fallback.

https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/net/smsc,lan9115.yaml#L18

https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/net/smsc,lan9115.yaml#L102

https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/smsc/smsc911x.c#L2652

Best regards,
Parthiban V
> 
>          Andrew