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

Conor Dooley
2024-04-30T16:55:28Z

[-- Attachment #1: Type: text/plain, Size: 2929 bytes --]

On Tue, Apr 30, 2024 at 01:30:22PM +0000, Parthiban.Veerasooran@microchip.com 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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]