| Message ID | 20240418125648.372526-12-Parthiban.Veerasooran@microchip.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Add support for OPEN Alliance 10BASE-T1x MACPHY Serial Interface | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/apply | fail | Patch does not apply to net-next-1 |
> +/* OPEN Alliance Configuration Register #0 */ > +#define OA_TC6_REG_CONFIG0 0x0004 > +#define CONFIG0_ZARFE_ENABLE BIT(12) If this is a standard register, you should put these defined where other drivers can use them. > +static int lan865x_set_mac_address(struct net_device *netdev, void *addr) > +{ > + struct lan865x_priv *priv = netdev_priv(netdev); > + struct sockaddr *address = addr; > + int ret; > + > + ret = eth_prepare_mac_addr_change(netdev, addr); > + if (ret < 0) > + return ret; > + > + if (ether_addr_equal(address->sa_data, netdev->dev_addr)) > + return 0; > + > + ret = lan865x_set_hw_macaddr(priv, address->sa_data); > + if (ret) > + return ret; > + > + eth_hw_addr_set(netdev, address->sa_data); It seems more normal to call eth_commit_mac_addr_change(), which better pairs with eth_prepare_mac_addr_change(). > +static int lan865x_set_zarfe(struct lan865x_priv *priv) > +{ > + u32 regval; > + int ret; > + > + ret = oa_tc6_read_register(priv->tc6, OA_TC6_REG_CONFIG0, ®val); > + if (ret) > + return ret; > + > + /* Set Zero-Align Receive Frame Enable */ > + regval |= CONFIG0_ZARFE_ENABLE; > + > + return oa_tc6_write_register(priv->tc6, OA_TC6_REG_CONFIG0, regval); > +} There does not appear to be anything specific to your device here. So please make this a helper in the shared code, so any driver can use it. Andrew
Hi Andrew, On 24/04/24 5:57 am, Andrew Lunn wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > >> +/* OPEN Alliance Configuration Register #0 */ >> +#define OA_TC6_REG_CONFIG0 0x0004 >> +#define CONFIG0_ZARFE_ENABLE BIT(12) > > If this is a standard register, you should put these defined where > other drivers can use them. OK. Will move it to oa_tc6.c in the next version as you have also suggested to use this as a helper function in the below comment. > >> +static int lan865x_set_mac_address(struct net_device *netdev, void *addr) >> +{ >> + struct lan865x_priv *priv = netdev_priv(netdev); >> + struct sockaddr *address = addr; >> + int ret; >> + >> + ret = eth_prepare_mac_addr_change(netdev, addr); >> + if (ret < 0) >> + return ret; >> + >> + if (ether_addr_equal(address->sa_data, netdev->dev_addr)) >> + return 0; >> + >> + ret = lan865x_set_hw_macaddr(priv, address->sa_data); >> + if (ret) >> + return ret; >> + >> + eth_hw_addr_set(netdev, address->sa_data); > > It seems more normal to call eth_commit_mac_addr_change(), which > better pairs with eth_prepare_mac_addr_change(). OK, so I will replace eth_hw_addr_set() with eth_prepare_mac_addr_change() in the next version. > >> +static int lan865x_set_zarfe(struct lan865x_priv *priv) >> +{ >> + u32 regval; >> + int ret; >> + >> + ret = oa_tc6_read_register(priv->tc6, OA_TC6_REG_CONFIG0, ®val); >> + if (ret) >> + return ret; >> + >> + /* Set Zero-Align Receive Frame Enable */ >> + regval |= CONFIG0_ZARFE_ENABLE; >> + >> + return oa_tc6_write_register(priv->tc6, OA_TC6_REG_CONFIG0, regval); >> +} > > There does not appear to be anything specific to your device here. So > please make this a helper in the shared code, so any driver can use > it. OK, I will implement this helper function as oa_tc6_enable_zarfe() in the oa_tc6.c. Also do you want me to move this helper function implementation to a new patch? Best regards, Parthiban V > > Andrew >
> OK, I will implement this helper function as oa_tc6_enable_zarfe() in > the oa_tc6.c. Also do you want me to move this helper function > implementation to a new patch? Yes please. Andrew
Hi,
For me the mac driver fails to probe with the following log
[ 0.123325] SPI driver lan865x has no spi_device_id for microchip,lan8651
With this change the driver probes
diff --git a/drivers/net/ethernet/microchip/lan865x/lan865x.c b/drivers/net/ethernet/microchip/lan865x/lan865x.c
index 9abefa8b9d9f..72a663f14f50 100644
--- a/drivers/net/ethernet/microchip/lan865x/lan865x.c
+++ b/drivers/net/ethernet/microchip/lan865x/lan865x.c
@@ -364,7 +364,7 @@ static void lan865x_remove(struct spi_device *spi)
}
static const struct of_device_id lan865x_dt_ids[] = {
- { .compatible = "microchip,lan8651", "microchip,lan8650" },
+ { .compatible = "microchip,lan865x", "microchip,lan8650" },
{ /* Sentinel */ }
};
MODULE_DEVICE_TABLE(of, lan865x_dt_ids);
Along with compatible = "microchip,lan865x" in the dts
I'm running a dual lan8650 setup, neither IC passed the sw reset in the
oa_tc.c module, I need to pull the reset pin low to reset the pin before
the rest of the init stuff happens.
The datasheet recommends not doing a sw reset, excerpt from section
4.1.1.3 Software Reset
"Note: The SW_RESET bit of the Clause 22 Basic Control register will reset only the internal PHY, not
the entire device. This PHY only reset is not recommended for use. If such a reset is detected, by
reading the RESETC bit of the STS2 register, reset the entire device."
Doing a hw reset followed by a sw reset seems to work fine though. I
added the folloing patch to get things moving.
diff --git a/drivers/net/ethernet/microchip/lan865x/lan865x.c b/drivers/net/ethernet/microchip/lan865x/lan865x.c
index 72a663f14f50..993c4f9dec7e 100644
--- a/drivers/net/ethernet/microchip/lan865x/lan865x.c
+++ b/drivers/net/ethernet/microchip/lan865x/lan865x.c
@@ -9,6 +9,7 @@
#include <linux/kernel.h>
#include <linux/phy.h>
#include <linux/oa_tc6.h>
+#include <linux/gpio/driver.h>
#define DRV_NAME "lan865x"
@@ -36,6 +37,7 @@ struct lan865x_priv {
struct net_device *netdev;
struct spi_device *spi;
struct oa_tc6 *tc6;
+ struct gpio_desc *reset_gpio;
};
static int lan865x_set_hw_macaddr_low_bytes(struct oa_tc6 *tc6, const u8 *mac)
@@ -283,6 +285,29 @@ static int lan865x_set_zarfe(struct lan865x_priv *priv)
return oa_tc6_write_register(priv->tc6, OA_TC6_REG_CONFIG0, regval);
}
+static int lan865x_probe_reset_gpio(struct lan865x_priv *priv)
+{
+ priv->reset_gpio = devm_gpiod_get_optional(&priv->spi->dev,
+ "reset",
+ GPIOD_OUT_HIGH);
+ if (IS_ERR(priv->reset_gpio)) {
+ dev_err(&priv->spi->dev, "failed to parse reset gpio from dt");
+ return PTR_ERR(priv->reset_gpio);
+ }
+
+ return 0;
+}
+
+static void lan865x_hw_reset(struct lan865x_priv *priv)
+{
+ dev_info(&priv->spi->dev, "resetting device");
+ gpiod_set_value(priv->reset_gpio, 1);
+ // the datasheet specifies a minimum 5µs hold time
+ usleep_range(5,10);
+ gpiod_set_value(priv->reset_gpio, 0);
+ dev_info(&priv->spi->dev, "reset completed");
+}
+
static int lan865x_probe(struct spi_device *spi)
{
struct net_device *netdev;
@@ -297,6 +322,9 @@ static int lan865x_probe(struct spi_device *spi)
priv->netdev = netdev;
priv->spi = spi;
spi_set_drvdata(spi, priv);
+ lan865x_probe_reset_gpio(priv);
+ if(priv->reset_gpio)
+ lan865x_hw_reset(priv);
INIT_WORK(&priv->multicast_work, lan865x_multicast_work_handler);
priv->tc6 = oa_tc6_init(spi, netdev);
Since the chip does have a HW reset pin I think it would be nice to
at least expose this as an optional dt binding.
Maybe ignore the prints I forgot to remove :)
On Sat, Apr 27, 2024 at 09:19:34PM +0200, Ramón Nordin Rodriguez wrote: > Hi, > > For me the mac driver fails to probe with the following log > [ 0.123325] SPI driver lan865x has no spi_device_id for microchip,lan8651 > > With this change the driver probes > > diff --git a/drivers/net/ethernet/microchip/lan865x/lan865x.c b/drivers/net/ethernet/microchip/lan865x/lan865x.c > index 9abefa8b9d9f..72a663f14f50 100644 > --- a/drivers/net/ethernet/microchip/lan865x/lan865x.c > +++ b/drivers/net/ethernet/microchip/lan865x/lan865x.c > @@ -364,7 +364,7 @@ static void lan865x_remove(struct spi_device *spi) > } > > static const struct of_device_id lan865x_dt_ids[] = { > - { .compatible = "microchip,lan8651", "microchip,lan8650" }, Huh, that's very strange. I don't see a single instance in the tree of a of_device_id struct like this with two compatibles like this (at least with a search of `rg "\.compatible.*\", \"" drivers/`. Given the fallbacks in the binding, only "microchip,lan8650" actually needs to be here. > + { .compatible = "microchip,lan865x", "microchip,lan8650" }, > { /* Sentinel */ } > }; > MODULE_DEVICE_TABLE(of, lan865x_dt_ids); > > Along with compatible = "microchip,lan865x" in the dts Just to be clear, the compatible w/ an x is unacceptable due to the wildcard and the binding should stay as-is. Whatever probing bugs the code has need to be resolved instead :) Thanks, Conor.
On Sat, Apr 27, 2024 at 08:57:43PM +0100, Conor Dooley wrote: > > static const struct of_device_id lan865x_dt_ids[] = { > > - { .compatible = "microchip,lan8651", "microchip,lan8650" }, > > Huh, that's very strange. I don't see a single instance in the tree of a > of_device_id struct like this with two compatibles like this (at least > with a search of `rg "\.compatible.*\", \"" drivers/`. > > Given the fallbacks in the binding, only "microchip,lan8650" actually > needs to be here. > > > + { .compatible = "microchip,lan865x", "microchip,lan8650" }, > > { /* Sentinel */ } > > }; > > MODULE_DEVICE_TABLE(of, lan865x_dt_ids); > > > > Along with compatible = "microchip,lan865x" in the dts > > Just to be clear, the compatible w/ an x is unacceptable due to the > wildcard and the binding should stay as-is. Whatever probing bugs > the code has need to be resolved instead :) > All right, so when I change to @@ -364,7 +364,7 @@ static void lan865x_remove(struct spi_device *spi) } static const struct of_device_id lan865x_dt_ids[] = { - { .compatible = "microchip,lan8651", "microchip,lan8650" }, + { .compatible = "microchip,lan8650" }, { /* Sentinel */ } }; MODULE_DEVICE_TABLE(of, lan865x_dt_ids); I still get the output [ 0.124266] SPI driver lan865x has no spi_device_id for microchip,lan8650 But the driver does probe and I get a network interface. If no one beats me to it I'll single step the probe tomorrow. R
On Sat, Apr 27, 2024 at 10:13:33PM +0200, Ramón Nordin Rodriguez wrote: > On Sat, Apr 27, 2024 at 08:57:43PM +0100, Conor Dooley wrote: > > > static const struct of_device_id lan865x_dt_ids[] = { > > > - { .compatible = "microchip,lan8651", "microchip,lan8650" }, > > > > Huh, that's very strange. I don't see a single instance in the tree of a > > of_device_id struct like this with two compatibles like this (at least > > with a search of `rg "\.compatible.*\", \"" drivers/`. > > > > Given the fallbacks in the binding, only "microchip,lan8650" actually > > needs to be here. > > > > > + { .compatible = "microchip,lan865x", "microchip,lan8650" }, > > > { /* Sentinel */ } > > > }; > > > MODULE_DEVICE_TABLE(of, lan865x_dt_ids); > > > > > > Along with compatible = "microchip,lan865x" in the dts > > > > Just to be clear, the compatible w/ an x is unacceptable due to the > > wildcard and the binding should stay as-is. Whatever probing bugs > > the code has need to be resolved instead :) > > > > All right, so when I change to > > @@ -364,7 +364,7 @@ static void lan865x_remove(struct spi_device *spi) > } > > static const struct of_device_id lan865x_dt_ids[] = { > - { .compatible = "microchip,lan8651", "microchip,lan8650" }, > + { .compatible = "microchip,lan8650" }, > { /* Sentinel */ } > }; > MODULE_DEVICE_TABLE(of, lan865x_dt_ids); > > I still get the output > [ 0.124266] SPI driver lan865x has no spi_device_id for microchip,lan8650 > But the driver does probe and I get a network interface. > > If no one beats me to it I'll single step the probe tomorrow. I think the error pretty much is what it says it is, the driver doesn't appear to have a spi_device_id table containing lan8650. The name of the driver is lan685x which is used in the fallback clause in __spi_register_driver(), so it complains as it does not find lan8650 in either. If my understanding is correct, either a spi_device_id table is required or the driver needs a rename with s/x/0/. Cheers, Conor.
On Sat, Apr 27, 2024 at 09:19:34PM +0200, Ramón Nordin Rodriguez wrote: > Hi, > > For me the mac driver fails to probe with the following log > [ 0.123325] SPI driver lan865x has no spi_device_id for microchip,lan8651 > > With this change the driver probes > > diff --git a/drivers/net/ethernet/microchip/lan865x/lan865x.c b/drivers/net/ethernet/microchip/lan865x/lan865x.c > index 9abefa8b9d9f..72a663f14f50 100644 > --- a/drivers/net/ethernet/microchip/lan865x/lan865x.c > +++ b/drivers/net/ethernet/microchip/lan865x/lan865x.c > @@ -364,7 +364,7 @@ static void lan865x_remove(struct spi_device *spi) > } > > static const struct of_device_id lan865x_dt_ids[] = { > - { .compatible = "microchip,lan8651", "microchip,lan8650" }, > + { .compatible = "microchip,lan865x", "microchip,lan8650" }, > { /* Sentinel */ } > }; The device tree binding says: + compatible: + oneOf: + - const: microchip,lan8650 + - items: + - const: microchip,lan8651 + - const: microchip,lan8650 So your DT node should either be: compatible = "microchip,lan8651", "microchip,lan8650"; or compatible = "microchip,lan8650" There is no mention of lan865x in the binding, so this patch is clearly wrong. What do you have in your DT node? Andrew
On Sat, Apr 27, 2024 at 09:35:06PM +0200, Ramón Nordin Rodriguez wrote: > I'm running a dual lan8650 setup, neither IC passed the sw reset in the > oa_tc.c module, I need to pull the reset pin low to reset the pin before > the rest of the init stuff happens. > > The datasheet recommends not doing a sw reset, excerpt from section > 4.1.1.3 Software Reset > "Note: The SW_RESET bit of the Clause 22 Basic Control register will reset only the internal PHY, not > the entire device. This PHY only reset is not recommended for use. If such a reset is detected, by > reading the RESETC bit of the STS2 register, reset the entire device." That is not so good. The PHY driver does not know the PHY is embedded within another device. It has no idea of RESETC bit in STS2. Looking at the phy driver, i don't actually seeing it using genphy_soft_reset(). Do you see a code path where this could actually be an issue? Supporting a hardware reset does however make sense. It would be best if you submitted a proper clean patch. It can be added to the end of this series, keeping you as author. Andrew
Ok this tripped me up. > The device tree binding says: > > + compatible: > + oneOf: > + - const: microchip,lan8650 > + - items: > + - const: microchip,lan8651 > + - const: microchip,lan8650 > > So your DT node should either be: > > compatible = "microchip,lan8651", "microchip,lan8650"; > > or > > compatible = "microchip,lan8650" > > There is no mention of lan865x in the binding, so this patch is > clearly wrong. > > What do you have in your DT node? Initially I set compatible = "microchip,lan8650", and did not get the driver to probe, so I got carried away with adding things that were not necessary. I dropped my patch and tested again. What does work is setting: compatible = "microchip,lan8651" - or - compatible = "microchip,lan8651", "microchip,lan8650" but just compatible = "lan8650" does not work. Also I'm getting the output [ 0.125056] SPI driver lan8650 has no spi_device_id for microchip,lan8651 As Conor pointed out setting the define DRV_NAME to "lan8651" fixes that. Setting the define to "lan8650" yet gets the spi module to log the 'no spi_device id..'. I don't really have an opinion here, but I think there is a risk that more than one dev might stumble on the same thing as me and expect that either or should work. BR R
> I think the error pretty much is what it says it is, the driver doesn't > appear to have a spi_device_id table containing lan8650. The name of > the driver is lan685x which is used in the fallback clause in > __spi_register_driver(), so it complains as it does not find lan8650 in > either. If my understanding is correct, either a spi_device_id table is > required or the driver needs a rename with s/x/0/. > Right you are, no gdb necessary. With the caveat that I only get it working when setting DRV_NAME to "lan8651", setting it to "lan8650" still produces the log R
On Sat, Apr 27, 2024 at 10:58:07PM +0200, Andrew Lunn wrote: > On Sat, Apr 27, 2024 at 09:35:06PM +0200, Ramón Nordin Rodriguez wrote: > > I'm running a dual lan8650 setup, neither IC passed the sw reset in the > > oa_tc.c module, I need to pull the reset pin low to reset the pin before > > the rest of the init stuff happens. > > > > The datasheet recommends not doing a sw reset, excerpt from section > > 4.1.1.3 Software Reset > > "Note: The SW_RESET bit of the Clause 22 Basic Control register will reset only the internal PHY, not > > the entire device. This PHY only reset is not recommended for use. If such a reset is detected, by > > reading the RESETC bit of the STS2 register, reset the entire device." > > That is not so good. The PHY driver does not know the PHY is embedded > within another device. It has no idea of RESETC bit in STS2. Looking > at the phy driver, i don't actually seeing it using > genphy_soft_reset(). Do you see a code path where this could actually > be an issue? > I agree with your assesment, the phy won't reset itself, but maybe we could add some comment doc about not adding it for the lan8670, so no one trips over that in the future. Though the phy does not have to be baked with the lan8650 mac, so that might be tricky to cover all future bases there. But for now I don't think it matters. Then regarding doing the soft reset in the oa_tc6 module, I'm not sure that it matters, since it seems to work fine for me for as long as I do the hw reset first. But I can submit a suggestion for how to deal with reset-quirks if we want to have the soft reset optional. I'll run a test with and without the soft reset and see if I can spot any change in behaviour. Let me know if I missed any nuance in your question. > Supporting a hardware reset does however make sense. It would be best > if you submitted a proper clean patch. It can be added to the end of > this series, keeping you as author. > I'd be happy to. Getting late in scandinavia so I'll clean it up and submit tomorrow. R
> but just compatible = "lan8650" does not work.
The device tree binding says that is valid, so this needs fixing.
Maybe copy max1111.c in hwmon:
static const struct spi_device_id max1111_ids[] = {
{ "max1110", max1110 },
{ "max1111", max1111 },
{ "max1112", max1112 },
{ "max1113", max1113 },
{ },
};
MODULE_DEVICE_TABLE(spi, max1111_ids);
static struct spi_driver max1111_driver = {
.driver = {
.name = "max1111",
},
.id_table = max1111_ids,
.probe = max1111_probe,
.remove = max1111_remove,
};
Interestingly, there is no compatible table. So the device tree
binding probably should change, not require two compatibles for
lan8651.
Andrew
> I agree with your assesment, the phy won't reset itself, but maybe we > could add some comment doc about not adding it for the lan8670, > so no one trips over that in the future. In the PHY driver, you can provide your own .soft_reset handler. It could return -EOPNOTSUPP, or -EINVAL. Maybe check the data sheets for the standalone devices supported by the driver. Can you limit this to just the TC6 PHY? Andrew
On Sun, Apr 28, 2024 at 04:25:28PM +0200, Andrew Lunn wrote: > > I agree with your assesment, the phy won't reset itself, but maybe we > > could add some comment doc about not adding it for the lan8670, > > so no one trips over that in the future. > > In the PHY driver, you can provide your own .soft_reset handler. It > could return -EOPNOTSUPP, or -EINVAL. Maybe check the data sheets for > the standalone devices supported by the driver. Can you limit this to > just the TC6 PHY? > Gotcha, I think that should be pretty easy to handle then. The microchip_t1s.c module handles two phy families * lan865x - baked in * lan867x - standalone I need to do some thinking and get a bit more oriented. But pretty sure there is a simple path for this. R
Hi Andrew, On 26/04/24 11:44 pm, Andrew Lunn wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > >> OK, I will implement this helper function as oa_tc6_enable_zarfe() in >> the oa_tc6.c. Also do you want me to move this helper function >> implementation to a new patch? > > Yes please. OK. Thanks. Best regards, Parthiban V > > Andrew
Hi All, On 28/04/24 1:27 am, Conor Dooley wrote: > On Sat, Apr 27, 2024 at 09:19:34PM +0200, Ramón Nordin Rodriguez wrote: >> Hi, >> >> For me the mac driver fails to probe with the following log >> [ 0.123325] SPI driver lan865x has no spi_device_id for microchip,lan8651 >> >> With this change the driver probes >> >> diff --git a/drivers/net/ethernet/microchip/lan865x/lan865x.c b/drivers/net/ethernet/microchip/lan865x/lan865x.c >> index 9abefa8b9d9f..72a663f14f50 100644 >> --- a/drivers/net/ethernet/microchip/lan865x/lan865x.c >> +++ b/drivers/net/ethernet/microchip/lan865x/lan865x.c >> @@ -364,7 +364,7 @@ static void lan865x_remove(struct spi_device *spi) >> } >> >> static const struct of_device_id lan865x_dt_ids[] = { >> - { .compatible = "microchip,lan8651", "microchip,lan8650" }, > Huh, that's very strange. I don't see a single instance in the tree of a > of_device_id struct like this with two compatibles like this (at least > with a search of `rg "\.compatible.*\", \"" drivers/`. > > Given the fallbacks in the binding, only "microchip,lan8650" actually > needs to be here. > >> + { .compatible = "microchip,lan865x", "microchip,lan8650" }, >> { /* Sentinel */ } >> }; >> MODULE_DEVICE_TABLE(of, lan865x_dt_ids); >> >> Along with compatible = "microchip,lan865x" in the dts > Just to be clear, the compatible w/ an x is unacceptable due to the > wildcard and the binding should stay as-is. Whatever probing bugs > the code has need to be resolved instead
> 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. Andrew
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
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"?
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: >> 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"? 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
Hi Parthiban
I'm using v4 of the driver an switched from ipv4 to ipv6.
I recognized, that the Neighbor Discovery is not working as expected.
The reason for this is, that the Neighbor Solicitation packet is not recived.
This packet is sent with a Multicast MAC Address.
When I checked the code and compared it to the documentaton the calculation of the Address Hash is not ok.
See Chapter 6.4.6 Hash Addressing
Changing the function lan865x_hash fixed the problem
+static inline u32 getAddrBit(u8 addr[ETH_ALEN], u32 bit)
+{
+ return ((addr[bit/8]) >> (bit % 8)) & 1;
+}
+
static u32 lan865x_hash(u8 addr[ETH_ALEN])
{
- return (ether_crc(ETH_ALEN, addr) >> 26) & GENMASK(5, 0);
+ u32 hash_index = 0;
+ for (int i=0; i<6; i++)
+ {
+ u32 hash = 0;
+ for (int j=0; j<8; j++) {
+ hash ^= getAddrBit(addr, (j*6)+i);
+ }
+ hash_index |= (hash << i);
+ }
+ return hash_index;
}
also the fuction lan865x_set_specific_multicast_addr() must be fixed due to the overflow of the mask variable
static void lan865x_set_specific_multicast_addr(struct net_device *netdev)
@@ -301,15 +315,11 @@ static void lan865x_set_specific_multicast_addr(struct net_device *netdev)
netdev_for_each_mc_addr(ha, netdev) {
u32 bit_num = lan865x_hash(ha->addr);
- u32 mask = BIT(bit_num);
- /* 5th bit of the 6 bits hash value is used to determine which
- * bit to set in either a high or low hash register.
- */
- if (bit_num & BIT(5))
- hash_hi |= mask;
+ if (bit_num >= 32)
+ hash_hi |= (1 << (bit_num-32));
else
- hash_lo |= mask;
+ hash_lo |= (1 << bit_num);
}
/* Enabling specific multicast addresses */
I would be great if the fix can be included in the new version 5 of your driver.
Thanks a lot and best regards
Stefan
Am 2024-04-18T14:56:47.000+02:00 hat Parthiban Veerasooran <Parthiban.Veerasooran@microchip.com> geschrieben:
> 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. 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 is into the LAN8650/1. The PHY and
> MAC are connected via an internal Media Independent Interface (MII).
>
> Signed-off-by: Parthiban Veerasooran <Parthiban.Veerasooran@microchip.com>
> ---
> MAINTAINERS | 6 +
> drivers/net/ethernet/microchip/Kconfig | 1 +
> drivers/net/ethernet/microchip/Makefile | 1 +
> .../net/ethernet/microchip/lan865x/Kconfig | 19 +
> .../net/ethernet/microchip/lan865x/Makefile | 6 +
> .../net/ethernet/microchip/lan865x/lan865x.c | 384 ++++++++++++++++++
> 6 files changed, 417 insertions(+)
> create mode 100644 drivers/net/ethernet/microchip/lan865x/Kconfig
> create mode 100644 drivers/net/ethernet/microchip/lan865x/Makefile
> create mode 100644 drivers/net/ethernet/microchip/lan865x/lan865x.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 603528948f61..f41b7f2257d2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14374,6 +14374,12 @@ L: netdev@vger.kernel.org
> S: Maintained
> F: drivers/net/ethernet/microchip/lan743x_*
>
> +MICROCHIP LAN8650/1 10BASE-T1S MACPHY ETHERNET DRIVER
> +M: Parthiban Veerasooran <parthiban.veerasooran@microchip.com>
> +L: netdev@vger.kernel.org
> +S: Maintained
> +F: drivers/net/ethernet/microchip/lan865x/lan865x.c
> +
> MICROCHIP LAN87xx/LAN937x T1 PHY DRIVER
> M: Arun Ramadoss <arun.ramadoss@microchip.com>
> R: UNGLinuxDriver@microchip.com
> diff --git a/drivers/net/ethernet/microchip/Kconfig b/drivers/net/ethernet/microchip/Kconfig
> index 43ba71e82260..06ca79669053 100644
> --- a/drivers/net/ethernet/microchip/Kconfig
> +++ b/drivers/net/ethernet/microchip/Kconfig
> @@ -56,6 +56,7 @@ config LAN743X
> To compile this driver as a module, choose M here. The module will be
> called lan743x.
>
> +source "drivers/net/ethernet/microchip/lan865x/Kconfig"
> source "drivers/net/ethernet/microchip/lan966x/Kconfig"
> source "drivers/net/ethernet/microchip/sparx5/Kconfig"
> source "drivers/net/ethernet/microchip/vcap/Kconfig"
> diff --git a/drivers/net/ethernet/microchip/Makefile b/drivers/net/ethernet/microchip/Makefile
> index bbd349264e6f..15dfbb321057 100644
> --- a/drivers/net/ethernet/microchip/Makefile
> +++ b/drivers/net/ethernet/microchip/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_LAN743X) += lan743x.o
>
> lan743x-objs := lan743x_main.o lan743x_ethtool.o lan743x_ptp.o
>
> +obj-$(CONFIG_LAN865X) += lan865x/
> obj-$(CONFIG_LAN966X_SWITCH) += lan966x/
> obj-$(CONFIG_SPARX5_SWITCH) += sparx5/
> obj-$(CONFIG_VCAP) += vcap/
> diff --git a/drivers/net/ethernet/microchip/lan865x/Kconfig b/drivers/net/ethernet/microchip/lan865x/Kconfig
> new file mode 100644
> index 000000000000..f3d60d14e202
> --- /dev/null
> +++ b/drivers/net/ethernet/microchip/lan865x/Kconfig
> @@ -0,0 +1,19 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# Microchip LAN865x Driver Support
> +#
> +
> +if NET_VENDOR_MICROCHIP
> +
> +config LAN865X
> + tristate "LAN865x support"
> + depends on SPI
> + depends on OA_TC6
> + help
> + Support for the Microchip LAN8650/1 Rev.B1 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.
> +
> +endif # NET_VENDOR_MICROCHIP
> diff --git a/drivers/net/ethernet/microchip/lan865x/Makefile b/drivers/net/ethernet/microchip/lan865x/Makefile
> new file mode 100644
> index 000000000000..9f5dd89c1eb8
> --- /dev/null
> +++ b/drivers/net/ethernet/microchip/lan865x/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# Makefile for the Microchip LAN865x Driver
> +#
> +
> +obj-$(CONFIG_LAN865X) += lan865x.o
> diff --git a/drivers/net/ethernet/microchip/lan865x/lan865x.c b/drivers/net/ethernet/microchip/lan865x/lan865x.c
> new file mode 100644
> index 000000000000..9abefa8b9d9f
> --- /dev/null
> +++ b/drivers/net/ethernet/microchip/lan865x/lan865x.c
> @@ -0,0 +1,384 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Microchip's LAN865x 10BASE-T1S MAC-PHY driver
> + *
> + * Author: Parthiban Veerasooran <parthiban.veerasooran@microchip.com>
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/phy.h>
> +#include <linux/oa_tc6.h>
> +
> +#define DRV_NAME "lan865x"
> +
> +/* MAC Network Control Register */
> +#define LAN865X_REG_MAC_NET_CTL 0x00010000
> +#define MAC_NET_CTL_TXEN BIT(3) /* Transmit Enable */
> +#define MAC_NET_CTL_RXEN BIT(2) /* Receive Enable */
> +
> +#define LAN865X_REG_MAC_NET_CFG 0x00010001 /* MAC Network Configuration Reg */
> +#define MAC_NET_CFG_PROMISCUOUS_MODE BIT(4)
> +#define MAC_NET_CFG_MULTICAST_MODE BIT(6)
> +#define MAC_NET_CFG_UNICAST_MODE BIT(7)
> +
> +#define LAN865X_REG_MAC_L_HASH 0x00010020 /* MAC Hash Register Bottom */
> +#define LAN865X_REG_MAC_H_HASH 0x00010021 /* MAC Hash Register Top */
> +#define LAN865X_REG_MAC_L_SADDR1 0x00010022 /* MAC Specific Addr 1 Bottom Reg */
> +#define LAN865X_REG_MAC_H_SADDR1 0x00010023 /* MAC Specific Addr 1 Top Reg */
> +
> +/* OPEN Alliance Configuration Register #0 */
> +#define OA_TC6_REG_CONFIG0 0x0004
> +#define CONFIG0_ZARFE_ENABLE BIT(12)
> +
> +struct lan865x_priv {
> + struct work_struct multicast_work;
> + struct net_device *netdev;
> + struct spi_device *spi;
> + struct oa_tc6 *tc6;
> +};
> +
> +static int lan865x_set_hw_macaddr_low_bytes(struct oa_tc6 *tc6, const u8 *mac)
> +{
> + u32 regval;
> +
> + regval = (mac[3] << 24) | (mac[2] << 16) | (mac[1] << 8) | mac[0];
> +
> + return oa_tc6_write_register(tc6, LAN865X_REG_MAC_L_SADDR1, regval);
> +}
> +
> +static int lan865x_set_hw_macaddr(struct lan865x_priv *priv, const u8 *mac)
> +{
> + int restore_ret;
> + u32 regval;
> + int ret;
> +
> + /* Configure MAC address low bytes */
> + ret = lan865x_set_hw_macaddr_low_bytes(priv->tc6, mac);
> + if (ret)
> + return ret;
> +
> + /* Prepare and configure MAC address high bytes */
> + regval = (mac[5] << 8) | mac[4];
> + ret = oa_tc6_write_register(priv->tc6, LAN865X_REG_MAC_H_SADDR1, regval);
> + if (!ret)
> + return 0;
> +
> + /* Restore the old MAC address low bytes from netdev if the new MAC
> + * address high bytes setting failed.
> + */
> + restore_ret = lan865x_set_hw_macaddr_low_bytes(priv->tc6,
> + priv->netdev->dev_addr);
> + if (restore_ret)
> + return restore_ret;
> +
> + return ret;
> +}
> +
> +static void
> +lan865x_get_drvinfo(struct net_device *netdev, struct ethtool_drvinfo *info)
> +{
> + strscpy(info->driver, DRV_NAME, sizeof(info->driver));
> + strscpy(info->bus_info, dev_name(netdev->dev.parent),
> + sizeof(info->bus_info));
> +}
> +
> +static const struct ethtool_ops lan865x_ethtool_ops = {
> + .get_drvinfo = lan865x_get_drvinfo,
> + .get_link_ksettings = phy_ethtool_get_link_ksettings,
> + .set_link_ksettings = phy_ethtool_set_link_ksettings,
> +};
> +
> +static int lan865x_set_mac_address(struct net_device *netdev, void *addr)
> +{
> + struct lan865x_priv *priv = netdev_priv(netdev);
> + struct sockaddr *address = addr;
> + int ret;
> +
> + ret = eth_prepare_mac_addr_change(netdev, addr);
> + if (ret < 0)
> + return ret;
> +
> + if (ether_addr_equal(address->sa_data, netdev->dev_addr))
> + return 0;
> +
> + ret = lan865x_set_hw_macaddr(priv, address->sa_data);
> + if (ret)
> + return ret;
> +
> + eth_hw_addr_set(netdev, address->sa_data);
> +
> + return 0;
> +}
> +
> +static u32 lan865x_hash(u8 addr[ETH_ALEN])
> +{
> + return (ether_crc(ETH_ALEN, addr) >> 26) & GENMASK(5, 0);
> +}
> +
> +static void lan865x_set_specific_multicast_addr(struct net_device *netdev)
> +{
> + struct lan865x_priv *priv = netdev_priv(netdev);
> + struct netdev_hw_addr *ha;
> + u32 hash_lo = 0;
> + u32 hash_hi = 0;
> +
> + netdev_for_each_mc_addr(ha, netdev) {
> + u32 bit_num = lan865x_hash(ha->addr);
> + u32 mask = BIT(bit_num);
> +
> + /* 5th bit of the 6 bits hash value is used to determine which
> + * bit to set in either a high or low hash register.
> + */
> + if (bit_num & BIT(5))
> + hash_hi |= mask;
> + else
> + hash_lo |= mask;
> + }
> +
> + /* Enabling specific multicast addresses */
> + if (oa_tc6_write_register(priv->tc6, LAN865X_REG_MAC_H_HASH, hash_hi)) {
> + netdev_err(netdev, "Failed to write reg_hashh");
> + return;
> + }
> +
> + if (oa_tc6_write_register(priv->tc6, LAN865X_REG_MAC_L_HASH, hash_lo))
> + netdev_err(netdev, "Failed to write reg_hashl");
> +}
> +
> +static void lan865x_multicast_work_handler(struct work_struct *work)
> +{
> + struct lan865x_priv *priv = container_of(work, struct lan865x_priv,
> + multicast_work);
> + u32 regval = 0;
> +
> + if (priv->netdev->flags & IFF_PROMISC) {
> + /* Enabling promiscuous mode */
> + regval |= MAC_NET_CFG_PROMISCUOUS_MODE;
> + regval &= (~MAC_NET_CFG_MULTICAST_MODE);
> + regval &= (~MAC_NET_CFG_UNICAST_MODE);
> + } else if (priv->netdev->flags & IFF_ALLMULTI) {
> + /* Enabling all multicast mode */
> + regval &= (~MAC_NET_CFG_PROMISCUOUS_MODE);
> + regval |= MAC_NET_CFG_MULTICAST_MODE;
> + regval &= (~MAC_NET_CFG_UNICAST_MODE);
> + } else if (!netdev_mc_empty(priv->netdev)) {
> + lan865x_set_specific_multicast_addr(priv->netdev);
> + regval &= (~MAC_NET_CFG_PROMISCUOUS_MODE);
> + regval &= (~MAC_NET_CFG_MULTICAST_MODE);
> + regval |= MAC_NET_CFG_UNICAST_MODE;
> + } else {
> + /* enabling local mac address only */
> + if (oa_tc6_write_register(priv->tc6, LAN865X_REG_MAC_H_HASH, 0)) {
> + netdev_err(priv->netdev, "Failed to write reg_hashh");
> + return;
> + }
> + if (oa_tc6_write_register(priv->tc6, LAN865X_REG_MAC_L_HASH, 0)) {
> + netdev_err(priv->netdev, "Failed to write reg_hashl");
> + return;
> + }
> + }
> + if (oa_tc6_write_register(priv->tc6, LAN865X_REG_MAC_NET_CFG, regval))
> + netdev_err(priv->netdev,
> + "Failed to enable promiscuous/multicast/normal mode");
> +}
> +
> +static void lan865x_set_multicast_list(struct net_device *netdev)
> +{
> + struct lan865x_priv *priv = netdev_priv(netdev);
> +
> + schedule_work(&priv->multicast_work);
> +}
> +
> +static netdev_tx_t lan865x_send_packet(struct sk_buff *skb,
> + struct net_device *netdev)
> +{
> + struct lan865x_priv *priv = netdev_priv(netdev);
> +
> + return oa_tc6_start_xmit(priv->tc6, skb);
> +}
> +
> +static int lan865x_hw_disable(struct lan865x_priv *priv)
> +{
> + u32 regval;
> +
> + if (oa_tc6_read_register(priv->tc6, LAN865X_REG_MAC_NET_CTL, ®val))
> + return -ENODEV;
> +
> + regval &= ~(MAC_NET_CTL_TXEN | MAC_NET_CTL_RXEN);
> +
> + if (oa_tc6_write_register(priv->tc6, LAN865X_REG_MAC_NET_CTL, regval))
> + return -ENODEV;
> +
> + return 0;
> +}
> +
> +static int lan865x_net_close(struct net_device *netdev)
> +{
> + struct lan865x_priv *priv = netdev_priv(netdev);
> + int ret;
> +
> + netif_stop_queue(netdev);
> + phy_stop(netdev->phydev);
> + ret = lan865x_hw_disable(priv);
> + if (ret) {
> + netdev_err(netdev, "Failed to disable the hardware: %d\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int lan865x_hw_enable(struct lan865x_priv *priv)
> +{
> + u32 regval;
> +
> + if (oa_tc6_read_register(priv->tc6, LAN865X_REG_MAC_NET_CTL, ®val))
> + return -ENODEV;
> +
> + regval |= MAC_NET_CTL_TXEN | MAC_NET_CTL_RXEN;
> +
> + if (oa_tc6_write_register(priv->tc6, LAN865X_REG_MAC_NET_CTL, regval))
> + return -ENODEV;
> +
> + return 0;
> +}
> +
> +static int lan865x_net_open(struct net_device *netdev)
> +{
> + struct lan865x_priv *priv = netdev_priv(netdev);
> + int ret;
> +
> + ret = lan865x_hw_enable(priv);
> + if (ret) {
> + netdev_err(netdev, "Failed to enable hardware: %d\n", ret);
> + return ret;
> + }
> +
> + phy_start(netdev->phydev);
> +
> + return 0;
> +}
> +
> +static const struct net_device_ops lan865x_netdev_ops = {
> + .ndo_open = lan865x_net_open,
> + .ndo_stop = lan865x_net_close,
> + .ndo_start_xmit = lan865x_send_packet,
> + .ndo_set_rx_mode = lan865x_set_multicast_list,
> + .ndo_set_mac_address = lan865x_set_mac_address,
> +};
> +
> +static int lan865x_set_zarfe(struct lan865x_priv *priv)
> +{
> + u32 regval;
> + int ret;
> +
> + ret = oa_tc6_read_register(priv->tc6, OA_TC6_REG_CONFIG0, ®val);
> + if (ret)
> + return ret;
> +
> + /* Set Zero-Align Receive Frame Enable */
> + regval |= CONFIG0_ZARFE_ENABLE;
> +
> + return oa_tc6_write_register(priv->tc6, OA_TC6_REG_CONFIG0, regval);
> +}
> +
> +static int lan865x_probe(struct spi_device *spi)
> +{
> + struct net_device *netdev;
> + struct lan865x_priv *priv;
> + int ret;
> +
> + netdev = alloc_etherdev(sizeof(struct lan865x_priv));
> + if (!netdev)
> + return -ENOMEM;
> +
> + priv = netdev_priv(netdev);
> + priv->netdev = netdev;
> + priv->spi = spi;
> + spi_set_drvdata(spi, priv);
> + INIT_WORK(&priv->multicast_work, lan865x_multicast_work_handler);
> +
> + priv->tc6 = oa_tc6_init(spi, netdev);
> + if (!priv->tc6) {
> + ret = -ENODEV;
> + goto free_netdev;
> + }
> +
> + /* As per the point s3 in the below errata, SPI receive Ethernet frame
> + * transfer may halt when starting the next frame in the same data block
> + * (chunk) as the end of a previous frame. The RFA field should be
> + * configured to 01b or 10b for proper operation. In these modes, only
> + * one receive Ethernet frame will be placed in a single data block.
> + * When the RFA field is written to 01b, received frames will be forced
> + * to only start in the first word of the data block payload (SWO=0). As
> + * recommended, ZARFE bit in the OPEN Alliance CONFIG0 register is set
> + * to 1 for proper operation.
> + *
> + * https://ww1.microchip.com/downloads/aemDocuments/documents/AIS/ProductDocuments/Errata/LAN8650-1-Errata-80001075.pdf
> + */
> + ret = lan865x_set_zarfe(priv);
> + if (ret) {
> + dev_err(&spi->dev, "Failed to set ZARFE: %d\n", ret);
> + goto oa_tc6_exit;
> + }
> +
> + /* Get the MAC address from the SPI device tree node */
> + if (device_get_ethdev_address(&spi->dev, netdev))
> + eth_hw_addr_random(netdev);
> +
> + ret = lan865x_set_hw_macaddr(priv, netdev->dev_addr);
> + if (ret) {
> + dev_err(&spi->dev, "Failed to configure MAC: %d\n", ret);
> + goto oa_tc6_exit;
> + }
> +
> + netdev->if_port = IF_PORT_10BASET;
> + netdev->irq = spi->irq;
> + netdev->netdev_ops = &lan865x_netdev_ops;
> + netdev->ethtool_ops = &lan865x_ethtool_ops;
> +
> + ret = register_netdev(netdev);
> + if (ret) {
> + dev_err(&spi->dev, "Register netdev failed (ret = %d)", ret);
> + goto oa_tc6_exit;
> + }
> +
> + return 0;
> +
> +oa_tc6_exit:
> + oa_tc6_exit(priv->tc6);
> +free_netdev:
> + free_netdev(priv->netdev);
> + return ret;
> +}
> +
> +static void lan865x_remove(struct spi_device *spi)
> +{
> + struct lan865x_priv *priv = spi_get_drvdata(spi);
> +
> + cancel_work_sync(&priv->multicast_work);
> + unregister_netdev(priv->netdev);
> + oa_tc6_exit(priv->tc6);
> + free_netdev(priv->netdev);
> +}
> +
> +static const struct of_device_id lan865x_dt_ids[] = {
> + { .compatible = "microchip,lan8651", "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,
> +};
> +module_spi_driver(lan865x_driver);
> +
> +MODULE_DESCRIPTION(DRV_NAME " 10Base-T1S MACPHY Ethernet Driver");
> +MODULE_AUTHOR("Parthiban Veerasooran <parthiban.veerasooran@microchip.com>");
> +MODULE_LICENSE("GPL");
> --
> 2.34.1
Hi Stefan, Thanks a lot for finding this. Sure I will include the below fix in the v5 patch series. Unfortunately the preparation of the v5 patch series takes little more time. Will try to post v5 patch series as soon as possible. Please keep supporting. Best regards, Parthiban V On 16/07/24 11:05 am, Stefan Bigler wrote: > [You don't often get email from linux@bigler.io. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Hi Parthiban > > I'm using v4 of the driver an switched from ipv4 to ipv6. > I recognized, that the Neighbor Discovery is not working as expected. > The reason for this is, that the Neighbor Solicitation packet is not recived. > This packet is sent with a Multicast MAC Address. > When I checked the code and compared it to the documentaton the calculation of the Address Hash is not ok. > See Chapter 6.4.6 Hash Addressing > > Changing the function lan865x_hash fixed the problem > > +static inline u32 getAddrBit(u8 addr[ETH_ALEN], u32 bit) > +{ > + return ((addr[bit/8]) >> (bit % 8)) & 1; > +} > + > static u32 lan865x_hash(u8 addr[ETH_ALEN]) > { > - return (ether_crc(ETH_ALEN, addr) >> 26) & GENMASK(5, 0); > + u32 hash_index = 0; > + for (int i=0; i<6; i++) > + { > + u32 hash = 0; > + for (int j=0; j<8; j++) { > + hash ^= getAddrBit(addr, (j*6)+i); > + } > + hash_index |= (hash << i); > + } > + return hash_index; > } > > also the fuction lan865x_set_specific_multicast_addr() must be fixed due to the overflow of the mask variable > > static void lan865x_set_specific_multicast_addr(struct net_device *netdev) > @@ -301,15 +315,11 @@ static void lan865x_set_specific_multicast_addr(struct net_device *netdev) > > netdev_for_each_mc_addr(ha, netdev) { > u32 bit_num = lan865x_hash(ha->addr); > - u32 mask = BIT(bit_num); > > - /* 5th bit of the 6 bits hash value is used to determine which > - * bit to set in either a high or low hash register. > - */ > - if (bit_num & BIT(5)) > - hash_hi |= mask; > + if (bit_num >= 32) > + hash_hi |= (1 << (bit_num-32)); > else > - hash_lo |= mask; > + hash_lo |= (1 << bit_num); > } > > /* Enabling specific multicast addresses */ > > I would be great if the fix can be included in the new version 5 of your driver. > > Thanks a lot and best regards > Stefan > > > Am 2024-04-18T14:56:47.000+02:00 hat Parthiban Veerasooran <Parthiban.Veerasooran@microchip.com> geschrieben: > >> 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. 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 is into the LAN8650/1. The PHY and >> MAC are connected via an internal Media Independent Interface (MII). >> >> Signed-off-by: Parthiban Veerasooran <Parthiban.Veerasooran@microchip.com> >> --- >> MAINTAINERS | 6 + >> drivers/net/ethernet/microchip/Kconfig | 1 + >> drivers/net/ethernet/microchip/Makefile | 1 + >> .../net/ethernet/microchip/lan865x/Kconfig | 19 + >> .../net/ethernet/microchip/lan865x/Makefile | 6 + >> .../net/ethernet/microchip/lan865x/lan865x.c | 384 ++++++++++++++++++ >> 6 files changed, 417 insertions(+) >> create mode 100644 drivers/net/ethernet/microchip/lan865x/Kconfig >> create mode 100644 drivers/net/ethernet/microchip/lan865x/Makefile >> create mode 100644 drivers/net/ethernet/microchip/lan865x/lan865x.c >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 603528948f61..f41b7f2257d2 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -14374,6 +14374,12 @@ L: netdev@vger.kernel.org >> S: Maintained >> F: drivers/net/ethernet/microchip/lan743x_* >> >> +MICROCHIP LAN8650/1 10BASE-T1S MACPHY ETHERNET DRIVER >> +M: Parthiban Veerasooran <parthiban.veerasooran@microchip.com> >> +L: netdev@vger.kernel.org >> +S: Maintained >> +F: drivers/net/ethernet/microchip/lan865x/lan865x.c >> + >> MICROCHIP LAN87xx/LAN937x T1 PHY DRIVER >> M: Arun Ramadoss <arun.ramadoss@microchip.com> >> R: UNGLinuxDriver@microchip.com >> diff --git a/drivers/net/ethernet/microchip/Kconfig b/drivers/net/ethernet/microchip/Kconfig >> index 43ba71e82260..06ca79669053 100644 >> --- a/drivers/net/ethernet/microchip/Kconfig >> +++ b/drivers/net/ethernet/microchip/Kconfig >> @@ -56,6 +56,7 @@ config LAN743X >> To compile this driver as a module, choose M here. The module will be >> called lan743x. >> >> +source "drivers/net/ethernet/microchip/lan865x/Kconfig" >> source "drivers/net/ethernet/microchip/lan966x/Kconfig" >> source "drivers/net/ethernet/microchip/sparx5/Kconfig" >> source "drivers/net/ethernet/microchip/vcap/Kconfig" >> diff --git a/drivers/net/ethernet/microchip/Makefile b/drivers/net/ethernet/microchip/Makefile >> index bbd349264e6f..15dfbb321057 100644 >> --- a/drivers/net/ethernet/microchip/Makefile >> +++ b/drivers/net/ethernet/microchip/Makefile >> @@ -9,6 +9,7 @@ obj-$(CONFIG_LAN743X) += lan743x.o >> >> lan743x-objs := lan743x_main.o lan743x_ethtool.o lan743x_ptp.o >> >> +obj-$(CONFIG_LAN865X) += lan865x/ >> obj-$(CONFIG_LAN966X_SWITCH) += lan966x/ >> obj-$(CONFIG_SPARX5_SWITCH) += sparx5/ >> obj-$(CONFIG_VCAP) += vcap/ >> diff --git a/drivers/net/ethernet/microchip/lan865x/Kconfig b/drivers/net/ethernet/microchip/lan865x/Kconfig >> new file mode 100644 >> index 000000000000..f3d60d14e202 >> --- /dev/null >> +++ b/drivers/net/ethernet/microchip/lan865x/Kconfig >> @@ -0,0 +1,19 @@ >> +# SPDX-License-Identifier: GPL-2.0-only >> +# >> +# Microchip LAN865x Driver Support >> +# >> + >> +if NET_VENDOR_MICROCHIP >> + >> +config LAN865X >> + tristate "LAN865x support" >> + depends on SPI >> + depends on OA_TC6 >> + help >> + Support for the Microchip LAN8650/1 Rev.B1 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. >> + >> +endif # NET_VENDOR_MICROCHIP >> diff --git a/drivers/net/ethernet/microchip/lan865x/Makefile b/drivers/net/ethernet/microchip/lan865x/Makefile >> new file mode 100644 >> index 000000000000..9f5dd89c1eb8 >> --- /dev/null >> +++ b/drivers/net/ethernet/microchip/lan865x/Makefile >> @@ -0,0 +1,6 @@ >> +# SPDX-License-Identifier: GPL-2.0-only >> +# >> +# Makefile for the Microchip LAN865x Driver >> +# >> + >> +obj-$(CONFIG_LAN865X) += lan865x.o >> diff --git a/drivers/net/ethernet/microchip/lan865x/lan865x.c b/drivers/net/ethernet/microchip/lan865x/lan865x.c >> new file mode 100644 >> index 000000000000..9abefa8b9d9f >> --- /dev/null >> +++ b/drivers/net/ethernet/microchip/lan865x/lan865x.c >> @@ -0,0 +1,384 @@ >> +// SPDX-License-Identifier: GPL-2.0+ >> +/* >> + * Microchip's LAN865x 10BASE-T1S MAC-PHY driver >> + * >> + * Author: Parthiban Veerasooran <parthiban.veerasooran@microchip.com> >> + */ >> + >> +#include <linux/module.h> >> +#include <linux/kernel.h> >> +#include <linux/phy.h> >> +#include <linux/oa_tc6.h> >> + >> +#define DRV_NAME "lan865x" >> + >> +/* MAC Network Control Register */ >> +#define LAN865X_REG_MAC_NET_CTL 0x00010000 >> +#define MAC_NET_CTL_TXEN BIT(3) /* Transmit Enable */ >> +#define MAC_NET_CTL_RXEN BIT(2) /* Receive Enable */ >> + >> +#define LAN865X_REG_MAC_NET_CFG 0x00010001 /* MAC Network Configuration Reg */ >> +#define MAC_NET_CFG_PROMISCUOUS_MODE BIT(4) >> +#define MAC_NET_CFG_MULTICAST_MODE BIT(6) >> +#define MAC_NET_CFG_UNICAST_MODE BIT(7) >> + >> +#define LAN865X_REG_MAC_L_HASH 0x00010020 /* MAC Hash Register Bottom */ >> +#define LAN865X_REG_MAC_H_HASH 0x00010021 /* MAC Hash Register Top */ >> +#define LAN865X_REG_MAC_L_SADDR1 0x00010022 /* MAC Specific Addr 1 Bottom Reg */ >> +#define LAN865X_REG_MAC_H_SADDR1 0x00010023 /* MAC Specific Addr 1 Top Reg */ >> + >> +/* OPEN Alliance Configuration Register #0 */ >> +#define OA_TC6_REG_CONFIG0 0x0004 >> +#define CONFIG0_ZARFE_ENABLE BIT(12) >> + >> +struct lan865x_priv { >> + struct work_struct multicast_work; >> + struct net_device *netdev; >> + struct spi_device *spi; >> + struct oa_tc6 *tc6; >> +}; >> + >> +static int lan865x_set_hw_macaddr_low_bytes(struct oa_tc6 *tc6, const u8 *mac) >> +{ >> + u32 regval; >> + >> + regval = (mac[3] << 24) | (mac[2] << 16) | (mac[1] << 8) | mac[0]; >> + >> + return oa_tc6_write_register(tc6, LAN865X_REG_MAC_L_SADDR1, regval); >> +} >> + >> +static int lan865x_set_hw_macaddr(struct lan865x_priv *priv, const u8 *mac) >> +{ >> + int restore_ret; >> + u32 regval; >> + int ret; >> + >> + /* Configure MAC address low bytes */ >> + ret = lan865x_set_hw_macaddr_low_bytes(priv->tc6, mac); >> + if (ret) >> + return ret; >> + >> + /* Prepare and configure MAC address high bytes */ >> + regval = (mac[5] << 8) | mac[4]; >> + ret = oa_tc6_write_register(priv->tc6, LAN865X_REG_MAC_H_SADDR1, regval); >> + if (!ret) >> + return 0; >> + >> + /* Restore the old MAC address low bytes from netdev if the new MAC >> + * address high bytes setting failed. >> + */ >> + restore_ret = lan865x_set_hw_macaddr_low_bytes(priv->tc6, >> + priv->netdev->dev_addr); >> + if (restore_ret) >> + return restore_ret; >> + >> + return ret; >> +} >> + >> +static void >> +lan865x_get_drvinfo(struct net_device *netdev, struct ethtool_drvinfo *info) >> +{ >> + strscpy(info->driver, DRV_NAME, sizeof(info->driver)); >> + strscpy(info->bus_info, dev_name(netdev->dev.parent), >> + sizeof(info->bus_info)); >> +} >> + >> +static const struct ethtool_ops lan865x_ethtool_ops = { >> + .get_drvinfo = lan865x_get_drvinfo, >> + .get_link_ksettings = phy_ethtool_get_link_ksettings, >> + .set_link_ksettings = phy_ethtool_set_link_ksettings, >> +}; >> + >> +static int lan865x_set_mac_address(struct net_device *netdev, void *addr) >> +{ >> + struct lan865x_priv *priv = netdev_priv(netdev); >> + struct sockaddr *address = addr; >> + int ret; >> + >> + ret = eth_prepare_mac_addr_change(netdev, addr); >> + if (ret < 0) >> + return ret; >> + >> + if (ether_addr_equal(address->sa_data, netdev->dev_addr)) >> + return 0; >> + >> + ret = lan865x_set_hw_macaddr(priv, address->sa_data); >> + if (ret) >> + return ret; >> + >> + eth_hw_addr_set(netdev, address->sa_data); >> + >> + return 0; >> +} >> + >> +static u32 lan865x_hash(u8 addr[ETH_ALEN]) >> +{ >> + return (ether_crc(ETH_ALEN, addr) >> 26) & GENMASK(5, 0); >> +} >> + >> +static void lan865x_set_specific_multicast_addr(struct net_device *netdev) >> +{ >> + struct lan865x_priv *priv = netdev_priv(netdev); >> + struct netdev_hw_addr *ha; >> + u32 hash_lo = 0; >> + u32 hash_hi = 0; >> + >> + netdev_for_each_mc_addr(ha, netdev) { >> + u32 bit_num = lan865x_hash(ha->addr); >> + u32 mask = BIT(bit_num); >> + >> + /* 5th bit of the 6 bits hash value is used to determine which >> + * bit to set in either a high or low hash register. >> + */ >> + if (bit_num & BIT(5)) >> + hash_hi |= mask; >> + else >> + hash_lo |= mask; >> + } >> + >> + /* Enabling specific multicast addresses */ >> + if (oa_tc6_write_register(priv->tc6, LAN865X_REG_MAC_H_HASH, hash_hi)) { >> + netdev_err(netdev, "Failed to write reg_hashh"); >> + return; >> + } >> + >> + if (oa_tc6_write_register(priv->tc6, LAN865X_REG_MAC_L_HASH, hash_lo)) >> + netdev_err(netdev, "Failed to write reg_hashl"); >> +} >> + >> +static void lan865x_multicast_work_handler(struct work_struct *work) >> +{ >> + struct lan865x_priv *priv = container_of(work, struct lan865x_priv, >> + multicast_work); >> + u32 regval = 0; >> + >> + if (priv->netdev->flags & IFF_PROMISC) { >> + /* Enabling promiscuous mode */ >> + regval |= MAC_NET_CFG_PROMISCUOUS_MODE; >> + regval &= (~MAC_NET_CFG_MULTICAST_MODE); >> + regval &= (~MAC_NET_CFG_UNICAST_MODE); >> + } else if (priv->netdev->flags & IFF_ALLMULTI) { >> + /* Enabling all multicast mode */ >> + regval &= (~MAC_NET_CFG_PROMISCUOUS_MODE); >> + regval |= MAC_NET_CFG_MULTICAST_MODE; >> + regval &= (~MAC_NET_CFG_UNICAST_MODE); >> + } else if (!netdev_mc_empty(priv->netdev)) { >> + lan865x_set_specific_multicast_addr(priv->netdev); >> + regval &= (~MAC_NET_CFG_PROMISCUOUS_MODE); >> + regval &= (~MAC_NET_CFG_MULTICAST_MODE); >> + regval |= MAC_NET_CFG_UNICAST_MODE; >> + } else { >> + /* enabling local mac address only */ >> + if (oa_tc6_write_register(priv->tc6, LAN865X_REG_MAC_H_HASH, 0)) { >> + netdev_err(priv->netdev, "Failed to write reg_hashh"); >> + return; >> + } >> + if (oa_tc6_write_register(priv->tc6, LAN865X_REG_MAC_L_HASH, 0)) { >> + netdev_err(priv->netdev, "Failed to write reg_hashl"); >> + return; >> + } >> + } >> + if (oa_tc6_write_register(priv->tc6, LAN865X_REG_MAC_NET_CFG, regval)) >> + netdev_err(priv->netdev, >> + "Failed to enable promiscuous/multicast/normal mode"); >> +} >> + >> +static void lan865x_set_multicast_list(struct net_device *netdev) >> +{ >> + struct lan865x_priv *priv = netdev_priv(netdev); >> + >> + schedule_work(&priv->multicast_work); >> +} >> + >> +static netdev_tx_t lan865x_send_packet(struct sk_buff *skb, >> + struct net_device *netdev) >> +{ >> + struct lan865x_priv *priv = netdev_priv(netdev); >> + >> + return oa_tc6_start_xmit(priv->tc6, skb); >> +} >> + >> +static int lan865x_hw_disable(struct lan865x_priv *priv) >> +{ >> + u32 regval; >> + >> + if (oa_tc6_read_register(priv->tc6, LAN865X_REG_MAC_NET_CTL, ®val)) >> + return -ENODEV; >> + >> + regval &= ~(MAC_NET_CTL_TXEN | MAC_NET_CTL_RXEN); >> + >> + if (oa_tc6_write_register(priv->tc6, LAN865X_REG_MAC_NET_CTL, regval)) >> + return -ENODEV; >> + >> + return 0; >> +} >> + >> +static int lan865x_net_close(struct net_device *netdev) >> +{ >> + struct lan865x_priv *priv = netdev_priv(netdev); >> + int ret; >> + >> + netif_stop_queue(netdev); >> + phy_stop(netdev->phydev); >> + ret = lan865x_hw_disable(priv); >> + if (ret) { >> + netdev_err(netdev, "Failed to disable the hardware: %d\n", ret); >> + return ret; >> + } >> + >> + return 0; >> +} >> + >> +static int lan865x_hw_enable(struct lan865x_priv *priv) >> +{ >> + u32 regval; >> + >> + if (oa_tc6_read_register(priv->tc6, LAN865X_REG_MAC_NET_CTL, ®val)) >> + return -ENODEV; >> + >> + regval |= MAC_NET_CTL_TXEN | MAC_NET_CTL_RXEN; >> + >> + if (oa_tc6_write_register(priv->tc6, LAN865X_REG_MAC_NET_CTL, regval)) >> + return -ENODEV; >> + >> + return 0; >> +} >> + >> +static int lan865x_net_open(struct net_device *netdev) >> +{ >> + struct lan865x_priv *priv = netdev_priv(netdev); >> + int ret; >> + >> + ret = lan865x_hw_enable(priv); >> + if (ret) { >> + netdev_err(netdev, "Failed to enable hardware: %d\n", ret); >> + return ret; >> + } >> + >> + phy_start(netdev->phydev); >> + >> + return 0; >> +} >> + >> +static const struct net_device_ops lan865x_netdev_ops = { >> + .ndo_open = lan865x_net_open, >> + .ndo_stop = lan865x_net_close, >> + .ndo_start_xmit = lan865x_send_packet, >> + .ndo_set_rx_mode = lan865x_set_multicast_list, >> + .ndo_set_mac_address = lan865x_set_mac_address, >> +}; >> + >> +static int lan865x_set_zarfe(struct lan865x_priv *priv) >> +{ >> + u32 regval; >> + int ret; >> + >> + ret = oa_tc6_read_register(priv->tc6, OA_TC6_REG_CONFIG0, ®val); >> + if (ret) >> + return ret; >> + >> + /* Set Zero-Align Receive Frame Enable */ >> + regval |= CONFIG0_ZARFE_ENABLE; >> + >> + return oa_tc6_write_register(priv->tc6, OA_TC6_REG_CONFIG0, regval); >> +} >> + >> +static int lan865x_probe(struct spi_device *spi) >> +{ >> + struct net_device *netdev; >> + struct lan865x_priv *priv; >> + int ret; >> + >> + netdev = alloc_etherdev(sizeof(struct lan865x_priv)); >> + if (!netdev) >> + return -ENOMEM; >> + >> + priv = netdev_priv(netdev); >> + priv->netdev = netdev; >> + priv->spi = spi; >> + spi_set_drvdata(spi, priv); >> + INIT_WORK(&priv->multicast_work, lan865x_multicast_work_handler); >> + >> + priv->tc6 = oa_tc6_init(spi, netdev); >> + if (!priv->tc6) { >> + ret = -ENODEV; >> + goto free_netdev; >> + } >> + >> + /* As per the point s3 in the below errata, SPI receive Ethernet frame >> + * transfer may halt when starting the next frame in the same data block >> + * (chunk) as the end of a previous frame. The RFA field should be >> + * configured to 01b or 10b for proper operation. In these modes, only >> + * one receive Ethernet frame will be placed in a single data block. >> + * When the RFA field is written to 01b, received frames will be forced >> + * to only start in the first word of the data block payload (SWO=0). As >> + * recommended, ZARFE bit in the OPEN Alliance CONFIG0 register is set >> + * to 1 for proper operation. >> + * >> + * https://ww1.microchip.com/downloads/aemDocuments/documents/AIS/ProductDocuments/Errata/LAN8650-1-Errata-80001075.pdf >> + */ >> + ret = lan865x_set_zarfe(priv); >> + if (ret) { >> + dev_err(&spi->dev, "Failed to set ZARFE: %d\n", ret); >> + goto oa_tc6_exit; >> + } >> + >> + /* Get the MAC address from the SPI device tree node */ >> + if (device_get_ethdev_address(&spi->dev, netdev)) >> + eth_hw_addr_random(netdev); >> + >> + ret = lan865x_set_hw_macaddr(priv, netdev->dev_addr); >> + if (ret) { >> + dev_err(&spi->dev, "Failed to configure MAC: %d\n", ret); >> + goto oa_tc6_exit; >> + } >> + >> + netdev->if_port = IF_PORT_10BASET; >> + netdev->irq = spi->irq; >> + netdev->netdev_ops = &lan865x_netdev_ops; >> + netdev->ethtool_ops = &lan865x_ethtool_ops; >> + >> + ret = register_netdev(netdev); >> + if (ret) { >> + dev_err(&spi->dev, "Register netdev failed (ret = %d)", ret); >> + goto oa_tc6_exit; >> + } >> + >> + return 0; >> + >> +oa_tc6_exit: >> + oa_tc6_exit(priv->tc6); >> +free_netdev: >> + free_netdev(priv->netdev); >> + return ret; >> +} >> + >> +static void lan865x_remove(struct spi_device *spi) >> +{ >> + struct lan865x_priv *priv = spi_get_drvdata(spi); >> + >> + cancel_work_sync(&priv->multicast_work); >> + unregister_netdev(priv->netdev); >> + oa_tc6_exit(priv->tc6); >> + free_netdev(priv->netdev); >> +} >> + >> +static const struct of_device_id lan865x_dt_ids[] = { >> + { .compatible = "microchip,lan8651", "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, >> +}; >> +module_spi_driver(lan865x_driver); >> + >> +MODULE_DESCRIPTION(DRV_NAME " 10Base-T1S MACPHY Ethernet Driver"); >> +MODULE_AUTHOR("Parthiban Veerasooran <parthiban.veerasooran@microchip.com>"); >> +MODULE_LICENSE("GPL"); >> -- >> 2.34.1 >
diff --git a/MAINTAINERS b/MAINTAINERS index 603528948f61..f41b7f2257d2 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -14374,6 +14374,12 @@ L: netdev@vger.kernel.org S: Maintained F: drivers/net/ethernet/microchip/lan743x_* +MICROCHIP LAN8650/1 10BASE-T1S MACPHY ETHERNET DRIVER +M: Parthiban Veerasooran <parthiban.veerasooran@microchip.com> +L: netdev@vger.kernel.org +S: Maintained +F: drivers/net/ethernet/microchip/lan865x/lan865x.c + MICROCHIP LAN87xx/LAN937x T1 PHY DRIVER M: Arun Ramadoss <arun.ramadoss@microchip.com> R: UNGLinuxDriver@microchip.com diff --git a/drivers/net/ethernet/microchip/Kconfig b/drivers/net/ethernet/microchip/Kconfig index 43ba71e82260..06ca79669053 100644 --- a/drivers/net/ethernet/microchip/Kconfig +++ b/drivers/net/ethernet/microchip/Kconfig @@ -56,6 +56,7 @@ config LAN743X To compile this driver as a module, choose M here. The module will be called lan743x. +source "drivers/net/ethernet/microchip/lan865x/Kconfig" source "drivers/net/ethernet/microchip/lan966x/Kconfig" source "drivers/net/ethernet/microchip/sparx5/Kconfig" source "drivers/net/ethernet/microchip/vcap/Kconfig" diff --git a/drivers/net/ethernet/microchip/Makefile b/drivers/net/ethernet/microchip/Makefile index bbd349264e6f..15dfbb321057 100644 --- a/drivers/net/ethernet/microchip/Makefile +++ b/drivers/net/ethernet/microchip/Makefile @@ -9,6 +9,7 @@ obj-$(CONFIG_LAN743X) += lan743x.o lan743x-objs := lan743x_main.o lan743x_ethtool.o lan743x_ptp.o +obj-$(CONFIG_LAN865X) += lan865x/ obj-$(CONFIG_LAN966X_SWITCH) += lan966x/ obj-$(CONFIG_SPARX5_SWITCH) += sparx5/ obj-$(CONFIG_VCAP) += vcap/ diff --git a/drivers/net/ethernet/microchip/lan865x/Kconfig b/drivers/net/ethernet/microchip/lan865x/Kconfig new file mode 100644 index 000000000000..f3d60d14e202 --- /dev/null +++ b/drivers/net/ethernet/microchip/lan865x/Kconfig @@ -0,0 +1,19 @@ +# SPDX-License-Identifier: GPL-2.0-only +# +# Microchip LAN865x Driver Support +# + +if NET_VENDOR_MICROCHIP + +config LAN865X + tristate "LAN865x support" + depends on SPI + depends on OA_TC6 + help + Support for the Microchip LAN8650/1 Rev.B1 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. + +endif # NET_VENDOR_MICROCHIP diff --git a/drivers/net/ethernet/microchip/lan865x/Makefile b/drivers/net/ethernet/microchip/lan865x/Makefile new file mode 100644 index 000000000000..9f5dd89c1eb8 --- /dev/null +++ b/drivers/net/ethernet/microchip/lan865x/Makefile @@ -0,0 +1,6 @@ +# SPDX-License-Identifier: GPL-2.0-only +# +# Makefile for the Microchip LAN865x Driver +# + +obj-$(CONFIG_LAN865X) += lan865x.o diff --git a/drivers/net/ethernet/microchip/lan865x/lan865x.c b/drivers/net/ethernet/microchip/lan865x/lan865x.c new file mode 100644 index 000000000000..9abefa8b9d9f --- /dev/null +++ b/drivers/net/ethernet/microchip/lan865x/lan865x.c @@ -0,0 +1,384 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Microchip's LAN865x 10BASE-T1S MAC-PHY driver + * + * Author: Parthiban Veerasooran <parthiban.veerasooran@microchip.com> + */ + +#include <linux/module.h> +#include <linux/kernel.h> +#include <linux/phy.h> +#include <linux/oa_tc6.h> + +#define DRV_NAME "lan865x" + +/* MAC Network Control Register */ +#define LAN865X_REG_MAC_NET_CTL 0x00010000 +#define MAC_NET_CTL_TXEN BIT(3) /* Transmit Enable */ +#define MAC_NET_CTL_RXEN BIT(2) /* Receive Enable */ + +#define LAN865X_REG_MAC_NET_CFG 0x00010001 /* MAC Network Configuration Reg */ +#define MAC_NET_CFG_PROMISCUOUS_MODE BIT(4) +#define MAC_NET_CFG_MULTICAST_MODE BIT(6) +#define MAC_NET_CFG_UNICAST_MODE BIT(7) + +#define LAN865X_REG_MAC_L_HASH 0x00010020 /* MAC Hash Register Bottom */ +#define LAN865X_REG_MAC_H_HASH 0x00010021 /* MAC Hash Register Top */ +#define LAN865X_REG_MAC_L_SADDR1 0x00010022 /* MAC Specific Addr 1 Bottom Reg */ +#define LAN865X_REG_MAC_H_SADDR1 0x00010023 /* MAC Specific Addr 1 Top Reg */ + +/* OPEN Alliance Configuration Register #0 */ +#define OA_TC6_REG_CONFIG0 0x0004 +#define CONFIG0_ZARFE_ENABLE BIT(12) + +struct lan865x_priv { + struct work_struct multicast_work; + struct net_device *netdev; + struct spi_device *spi; + struct oa_tc6 *tc6; +}; + +static int lan865x_set_hw_macaddr_low_bytes(struct oa_tc6 *tc6, const u8 *mac) +{ + u32 regval; + + regval = (mac[3] << 24) | (mac[2] << 16) | (mac[1] << 8) | mac[0]; + + return oa_tc6_write_register(tc6, LAN865X_REG_MAC_L_SADDR1, regval); +} + +static int lan865x_set_hw_macaddr(struct lan865x_priv *priv, const u8 *mac) +{ + int restore_ret; + u32 regval; + int ret; + + /* Configure MAC address low bytes */ + ret = lan865x_set_hw_macaddr_low_bytes(priv->tc6, mac); + if (ret) + return ret; + + /* Prepare and configure MAC address high bytes */ + regval = (mac[5] << 8) | mac[4]; + ret = oa_tc6_write_register(priv->tc6, LAN865X_REG_MAC_H_SADDR1, regval); + if (!ret) + return 0; + + /* Restore the old MAC address low bytes from netdev if the new MAC + * address high bytes setting failed. + */ + restore_ret = lan865x_set_hw_macaddr_low_bytes(priv->tc6, + priv->netdev->dev_addr); + if (restore_ret) + return restore_ret; + + return ret; +} + +static void +lan865x_get_drvinfo(struct net_device *netdev, struct ethtool_drvinfo *info) +{ + strscpy(info->driver, DRV_NAME, sizeof(info->driver)); + strscpy(info->bus_info, dev_name(netdev->dev.parent), + sizeof(info->bus_info)); +} + +static const struct ethtool_ops lan865x_ethtool_ops = { + .get_drvinfo = lan865x_get_drvinfo, + .get_link_ksettings = phy_ethtool_get_link_ksettings, + .set_link_ksettings = phy_ethtool_set_link_ksettings, +}; + +static int lan865x_set_mac_address(struct net_device *netdev, void *addr) +{ + struct lan865x_priv *priv = netdev_priv(netdev); + struct sockaddr *address = addr; + int ret; + + ret = eth_prepare_mac_addr_change(netdev, addr); + if (ret < 0) + return ret; + + if (ether_addr_equal(address->sa_data, netdev->dev_addr)) + return 0; + + ret = lan865x_set_hw_macaddr(priv, address->sa_data); + if (ret) + return ret; + + eth_hw_addr_set(netdev, address->sa_data); + + return 0; +} + +static u32 lan865x_hash(u8 addr[ETH_ALEN]) +{ + return (ether_crc(ETH_ALEN, addr) >> 26) & GENMASK(5, 0); +} + +static void lan865x_set_specific_multicast_addr(struct net_device *netdev) +{ + struct lan865x_priv *priv = netdev_priv(netdev); + struct netdev_hw_addr *ha; + u32 hash_lo = 0; + u32 hash_hi = 0; + + netdev_for_each_mc_addr(ha, netdev) { + u32 bit_num = lan865x_hash(ha->addr); + u32 mask = BIT(bit_num); + + /* 5th bit of the 6 bits hash value is used to determine which + * bit to set in either a high or low hash register. + */ + if (bit_num & BIT(5)) + hash_hi |= mask; + else + hash_lo |= mask; + } + + /* Enabling specific multicast addresses */ + if (oa_tc6_write_register(priv->tc6, LAN865X_REG_MAC_H_HASH, hash_hi)) { + netdev_err(netdev, "Failed to write reg_hashh"); + return; + } + + if (oa_tc6_write_register(priv->tc6, LAN865X_REG_MAC_L_HASH, hash_lo)) + netdev_err(netdev, "Failed to write reg_hashl"); +} + +static void lan865x_multicast_work_handler(struct work_struct *work) +{ + struct lan865x_priv *priv = container_of(work, struct lan865x_priv, + multicast_work); + u32 regval = 0; + + if (priv->netdev->flags & IFF_PROMISC) { + /* Enabling promiscuous mode */ + regval |= MAC_NET_CFG_PROMISCUOUS_MODE; + regval &= (~MAC_NET_CFG_MULTICAST_MODE); + regval &= (~MAC_NET_CFG_UNICAST_MODE); + } else if (priv->netdev->flags & IFF_ALLMULTI) { + /* Enabling all multicast mode */ + regval &= (~MAC_NET_CFG_PROMISCUOUS_MODE); + regval |= MAC_NET_CFG_MULTICAST_MODE; + regval &= (~MAC_NET_CFG_UNICAST_MODE); + } else if (!netdev_mc_empty(priv->netdev)) { + lan865x_set_specific_multicast_addr(priv->netdev); + regval &= (~MAC_NET_CFG_PROMISCUOUS_MODE); + regval &= (~MAC_NET_CFG_MULTICAST_MODE); + regval |= MAC_NET_CFG_UNICAST_MODE; + } else { + /* enabling local mac address only */ + if (oa_tc6_write_register(priv->tc6, LAN865X_REG_MAC_H_HASH, 0)) { + netdev_err(priv->netdev, "Failed to write reg_hashh"); + return; + } + if (oa_tc6_write_register(priv->tc6, LAN865X_REG_MAC_L_HASH, 0)) { + netdev_err(priv->netdev, "Failed to write reg_hashl"); + return; + } + } + if (oa_tc6_write_register(priv->tc6, LAN865X_REG_MAC_NET_CFG, regval)) + netdev_err(priv->netdev, + "Failed to enable promiscuous/multicast/normal mode"); +} + +static void lan865x_set_multicast_list(struct net_device *netdev) +{ + struct lan865x_priv *priv = netdev_priv(netdev); + + schedule_work(&priv->multicast_work); +} + +static netdev_tx_t lan865x_send_packet(struct sk_buff *skb, + struct net_device *netdev) +{ + struct lan865x_priv *priv = netdev_priv(netdev); + + return oa_tc6_start_xmit(priv->tc6, skb); +} + +static int lan865x_hw_disable(struct lan865x_priv *priv) +{ + u32 regval; + + if (oa_tc6_read_register(priv->tc6, LAN865X_REG_MAC_NET_CTL, ®val)) + return -ENODEV; + + regval &= ~(MAC_NET_CTL_TXEN | MAC_NET_CTL_RXEN); + + if (oa_tc6_write_register(priv->tc6, LAN865X_REG_MAC_NET_CTL, regval)) + return -ENODEV; + + return 0; +} + +static int lan865x_net_close(struct net_device *netdev) +{ + struct lan865x_priv *priv = netdev_priv(netdev); + int ret; + + netif_stop_queue(netdev); + phy_stop(netdev->phydev); + ret = lan865x_hw_disable(priv); + if (ret) { + netdev_err(netdev, "Failed to disable the hardware: %d\n", ret); + return ret; + } + + return 0; +} + +static int lan865x_hw_enable(struct lan865x_priv *priv) +{ + u32 regval; + + if (oa_tc6_read_register(priv->tc6, LAN865X_REG_MAC_NET_CTL, ®val)) + return -ENODEV; + + regval |= MAC_NET_CTL_TXEN | MAC_NET_CTL_RXEN; + + if (oa_tc6_write_register(priv->tc6, LAN865X_REG_MAC_NET_CTL, regval)) + return -ENODEV; + + return 0; +} + +static int lan865x_net_open(struct net_device *netdev) +{ + struct lan865x_priv *priv = netdev_priv(netdev); + int ret; + + ret = lan865x_hw_enable(priv); + if (ret) { + netdev_err(netdev, "Failed to enable hardware: %d\n", ret); + return ret; + } + + phy_start(netdev->phydev); + + return 0; +} + +static const struct net_device_ops lan865x_netdev_ops = { + .ndo_open = lan865x_net_open, + .ndo_stop = lan865x_net_close, + .ndo_start_xmit = lan865x_send_packet, + .ndo_set_rx_mode = lan865x_set_multicast_list, + .ndo_set_mac_address = lan865x_set_mac_address, +}; + +static int lan865x_set_zarfe(struct lan865x_priv *priv) +{ + u32 regval; + int ret; + + ret = oa_tc6_read_register(priv->tc6, OA_TC6_REG_CONFIG0, ®val); + if (ret) + return ret; + + /* Set Zero-Align Receive Frame Enable */ + regval |= CONFIG0_ZARFE_ENABLE; + + return oa_tc6_write_register(priv->tc6, OA_TC6_REG_CONFIG0, regval); +} + +static int lan865x_probe(struct spi_device *spi) +{ + struct net_device *netdev; + struct lan865x_priv *priv; + int ret; + + netdev = alloc_etherdev(sizeof(struct lan865x_priv)); + if (!netdev) + return -ENOMEM; + + priv = netdev_priv(netdev); + priv->netdev = netdev; + priv->spi = spi; + spi_set_drvdata(spi, priv); + INIT_WORK(&priv->multicast_work, lan865x_multicast_work_handler); + + priv->tc6 = oa_tc6_init(spi, netdev); + if (!priv->tc6) { + ret = -ENODEV; + goto free_netdev; + } + + /* As per the point s3 in the below errata, SPI receive Ethernet frame + * transfer may halt when starting the next frame in the same data block + * (chunk) as the end of a previous frame. The RFA field should be + * configured to 01b or 10b for proper operation. In these modes, only + * one receive Ethernet frame will be placed in a single data block. + * When the RFA field is written to 01b, received frames will be forced + * to only start in the first word of the data block payload (SWO=0). As + * recommended, ZARFE bit in the OPEN Alliance CONFIG0 register is set + * to 1 for proper operation. + * + * https://ww1.microchip.com/downloads/aemDocuments/documents/AIS/ProductDocuments/Errata/LAN8650-1-Errata-80001075.pdf + */ + ret = lan865x_set_zarfe(priv); + if (ret) { + dev_err(&spi->dev, "Failed to set ZARFE: %d\n", ret); + goto oa_tc6_exit; + } + + /* Get the MAC address from the SPI device tree node */ + if (device_get_ethdev_address(&spi->dev, netdev)) + eth_hw_addr_random(netdev); + + ret = lan865x_set_hw_macaddr(priv, netdev->dev_addr); + if (ret) { + dev_err(&spi->dev, "Failed to configure MAC: %d\n", ret); + goto oa_tc6_exit; + } + + netdev->if_port = IF_PORT_10BASET; + netdev->irq = spi->irq; + netdev->netdev_ops = &lan865x_netdev_ops; + netdev->ethtool_ops = &lan865x_ethtool_ops; + + ret = register_netdev(netdev); + if (ret) { + dev_err(&spi->dev, "Register netdev failed (ret = %d)", ret); + goto oa_tc6_exit; + } + + return 0; + +oa_tc6_exit: + oa_tc6_exit(priv->tc6); +free_netdev: + free_netdev(priv->netdev); + return ret; +} + +static void lan865x_remove(struct spi_device *spi) +{ + struct lan865x_priv *priv = spi_get_drvdata(spi); + + cancel_work_sync(&priv->multicast_work); + unregister_netdev(priv->netdev); + oa_tc6_exit(priv->tc6); + free_netdev(priv->netdev); +} + +static const struct of_device_id lan865x_dt_ids[] = { + { .compatible = "microchip,lan8651", "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, +}; +module_spi_driver(lan865x_driver); + +MODULE_DESCRIPTION(DRV_NAME " 10Base-T1S MACPHY Ethernet Driver"); +MODULE_AUTHOR("Parthiban Veerasooran <parthiban.veerasooran@microchip.com>"); +MODULE_LICENSE("GPL");
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. 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 is into the LAN8650/1. The PHY and MAC are connected via an internal Media Independent Interface (MII). Signed-off-by: Parthiban Veerasooran <Parthiban.Veerasooran@microchip.com> --- MAINTAINERS | 6 + drivers/net/ethernet/microchip/Kconfig | 1 + drivers/net/ethernet/microchip/Makefile | 1 + .../net/ethernet/microchip/lan865x/Kconfig | 19 + .../net/ethernet/microchip/lan865x/Makefile | 6 + .../net/ethernet/microchip/lan865x/lan865x.c | 384 ++++++++++++++++++ 6 files changed, 417 insertions(+) create mode 100644 drivers/net/ethernet/microchip/lan865x/Kconfig create mode 100644 drivers/net/ethernet/microchip/lan865x/Makefile create mode 100644 drivers/net/ethernet/microchip/lan865x/lan865x.c