Hi Conor,

On 30/04/24 10:25 pm, Conor Dooley wrote:

On Tue, Apr 30, 2024 at 01:30:22PM +0000,Parthiban.Veerasooran@microchip.com  wrote:

quoted

Hi Andrew,

On 29/04/24 5:39 pm, Andrew Lunn wrote:

quoted

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

quoted

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"?

Do you mean changing in the lan865x.c file? if yes then I got the 
warning "SPI driver lan865x has no spi_device_id for microchip,lan8651"

But after updating the lan865x.c like below, the warning disappeared.

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

Note: In both the above two cases compatible in the dts is
compatible = "microchip,lan8651", "microchip,lan8650";

Best regards,
Parthiban V