[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <60fabe51-b682-479a-a528-74cf8af20309@microchip.com>
Date: Tue, 31 Oct 2023 10:25:57 +0000
From: <Parthiban.Veerasooran@...rochip.com>
To: <krzk@...nel.org>
CC: <netdev@...r.kernel.org>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <linux-doc@...r.kernel.org>,
<Horatiu.Vultur@...rochip.com>, <Woojung.Huh@...rochip.com>,
<Nicolas.Ferre@...rochip.com>, <UNGLinuxDriver@...rochip.com>,
<Thorsten.Kummermehr@...rochip.com>, <davem@...emloft.net>,
<edumazet@...gle.com>, <kuba@...nel.org>, <pabeni@...hat.com>,
<robh+dt@...nel.org>, <krzysztof.kozlowski+dt@...aro.org>,
<conor+dt@...nel.org>, <corbet@....net>, <Steen.Hegelund@...rochip.com>,
<rdunlap@...radead.org>, <horms@...nel.org>, <casper.casan@...il.com>,
<andrew@...n.ch>
Subject: Re: [PATCH net-next v2 8/9] microchip: lan865x: add driver support
for Microchip's LAN865X MACPHY
Hi Krzysztof,
On 24/10/23 5:27 pm, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On 23/10/2023 17:46, Parthiban Veerasooran wrote:
>> The LAN8650/1 is designed to conform to the OPEN Alliance 10BASE‑T1x
>> MAC‑PHY Serial Interface specification, Version 1.1. The IEEE Clause 4
>> MAC integration provides the low pin count standard SPI interface to any
>> microcontroller therefore providing Ethernet functionality without
>> requiring MAC integration within the microcontroller. The LAN8650/1
>> operates as an SPI client supporting SCLK clock rates up to a maximum of
>> 25 MHz. This SPI interface supports the transfer of both data (Ethernet
>> frames) and control (register access).
>>
>> By default, the chunk data payload is 64 bytes in size. A smaller payload
>> data size of 32 bytes is also supported and may be configured in the
>> Chunk Payload Size (CPS) field of the Configuration 0 (OA_CONFIG0)
>> register. Changing the chunk payload size requires the LAN8650/1 be reset
>> and shall not be done during normal operation.
>>
>> The Ethernet Media Access Controller (MAC) module implements a 10 Mbps
>> half duplex Ethernet MAC, compatible with the IEEE 802.3 standard.
>> 10BASE-T1S physical layer transceiver integrated into the LAN8650/1. The
>> PHY and MAC are connected via an internal Media Independent Interface
>> (MII).
>>
>> Signed-off-by: Parthiban Veerasooran <Parthiban.Veerasooran@...rochip.com>
>> ---
>> MAINTAINERS | 6 +
>> drivers/net/ethernet/microchip/Kconfig | 11 +
>> drivers/net/ethernet/microchip/Makefile | 2 +
>> drivers/net/ethernet/microchip/lan865x.c | 415 +++++++++++++++++++++++
>> 4 files changed, 434 insertions(+)
>> create mode 100644 drivers/net/ethernet/microchip/lan865x.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 9580be91f5e9..1b1bd3218a2d 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -14001,6 +14001,12 @@ L: netdev@...r.kernel.org
>> S: Maintained
>> F: drivers/net/ethernet/microchip/lan743x_*
>>
>> +MICROCHIP LAN8650/1 10BASE-T1S MACPHY ETHERNET DRIVER
>> +M: Parthiban Veerasooran <parthiban.veerasooran@...rochip.com>
>> +L: netdev@...r.kernel.org
>> +S: Maintained
>> +F: drivers/net/ethernet/microchip/lan865x.c
>> +
>> MICROCHIP LAN87xx/LAN937x T1 PHY DRIVER
>> M: Arun Ramadoss <arun.ramadoss@...rochip.com>
>> R: UNGLinuxDriver@...rochip.com
>> diff --git a/drivers/net/ethernet/microchip/Kconfig b/drivers/net/ethernet/microchip/Kconfig
>> index 329e374b9539..596caf59dea6 100644
>> --- a/drivers/net/ethernet/microchip/Kconfig
>> +++ b/drivers/net/ethernet/microchip/Kconfig
>> @@ -59,4 +59,15 @@ source "drivers/net/ethernet/microchip/lan966x/Kconfig"
>> source "drivers/net/ethernet/microchip/sparx5/Kconfig"
>> source "drivers/net/ethernet/microchip/vcap/Kconfig"
>>
>> +config LAN865X
>> + tristate "LAN865x support"
>> + depends on SPI
>> + depends on OA_TC6
>> + help
>> + Support for the Microchip LAN8650/1 Rev.B0 MACPHY Ethernet chip. It
>> + uses OPEN Alliance 10BASE-T1x Serial Interface specification.
>> +
>> + To compile this driver as a module, choose M here. The module will be
>> + called lan865x.
>
> That's odd indentation. Kconfig help goes with tab and two spaces.
Ah yes, will correct it.
>
>> +
>> endif # NET_VENDOR_MICROCHIP
>> diff --git a/drivers/net/ethernet/microchip/Makefile b/drivers/net/ethernet/microchip/Makefile
>> index bbd349264e6f..1fa4e15a067d 100644
>> --- a/drivers/net/ethernet/microchip/Makefile
>> +++ b/drivers/net/ethernet/microchip/Makefile
>> @@ -12,3 +12,5 @@ lan743x-objs := lan743x_main.o lan743x_ethtool.o lan743x_ptp.o
>> obj-$(CONFIG_LAN966X_SWITCH) += lan966x/
>> obj-$(CONFIG_SPARX5_SWITCH) += sparx5/
>> obj-$(CONFIG_VCAP) += vcap/
>
> ...
>
>> +static void lan865x_remove(struct spi_device *spi)
>> +{
>> + struct lan865x_priv *priv = spi_get_drvdata(spi);
>> +
>> + oa_tc6_exit(priv->tc6);
>> + unregister_netdev(priv->netdev);
>> + free_netdev(priv->netdev);
>> +}
>> +
>> +#ifdef CONFIG_OF
>
> Drop ifdef
Yes ok.
>
>> +static const struct of_device_id lan865x_dt_ids[] = {
>> + { .compatible = "microchip,lan865x" },
>> + { /* Sentinel */ }
>> +};
>> +MODULE_DEVICE_TABLE(of, lan865x_dt_ids);
>> +#endif
>> +
>> +#ifdef CONFIG_ACPI
I think I need to remove this ifdef as well?
>> +static const struct acpi_device_id lan865x_acpi_ids[] = {
>> + { .id = "LAN865X",
>> + },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(acpi, lan865x_acpi_ids);
>> +#endif
>> +
>> +static struct spi_driver lan865x_driver = {
>> + .driver = {
>> + .name = DRV_NAME,
>> +#ifdef CONFIG_OF
>
> Drop ifdef
Yes ok.
>
>> + .of_match_table = lan865x_dt_ids,
>> +#endif
>> +#ifdef CONFIG_ACPI
>
> Why do you need this ifdef?
Ya it is not needed. Will remove it.
>
>> + .acpi_match_table = ACPI_PTR(lan865x_acpi_ids),
>> +#endif
>> + },
>> + .probe = lan865x_probe,
>> + .remove = lan865x_remove,
>> +};
>> +module_spi_driver(lan865x_driver);
>> +
>> +MODULE_DESCRIPTION(DRV_NAME " 10Base-T1S MACPHY Ethernet Driver");
>> +MODULE_AUTHOR("Parthiban Veerasooran <parthiban.veerasooran@...rochip.com>");
>> +MODULE_LICENSE("GPL");
>> +MODULE_ALIAS("spi:" DRV_NAME);
>
> You should not need MODULE_ALIAS() in normal cases. If you need it,
> usually it means your device ID table is wrong.
Ok, will remove it.
Best Regards,
Parthiban V
>
>
> Best regards,
> Krzysztof
>
Powered by blists - more mailing lists