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

[Conor Dooley]

On Tue, Apr 30, 2024 at 01:30:22PM +0000, Parthiban.Veerasooran@xxxxxxxxxxxxx wrote:
> 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.

Did you also verify that the warning from the spi core is no longer
generated when using compatible = "microchip,lan8651", "microchip,lan8650"?

Attachment: signature.asc
Description: PGP signature