| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAJhJPsV4+WozxVb1=xQ6SGm5SVuKyk94Ns0nRz+kwB6Xh4C_WQ@mail.gmail.com>
Date: Mon, 21 Aug 2023 21:24:17 +0800
From: Keguang Zhang <keguang.zhang@...il.com>
To: Serge Semin <fancer.lancer@...il.com>
Cc: netdev@...r.kernel.org, devicetree@...r.kernel.org,
linux-mips@...r.kernel.org, linux-kernel@...r.kernel.org,
Lee Jones <lee@...nel.org>, Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>,
"David S . Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
Giuseppe Cavallaro <peppe.cavallaro@...com>,
Alexandre Torgue <alexandre.torgue@...s.st.com>,
Jose Abreu <joabreu@...opsys.com>,
Serge Semin <Sergey.Semin@...kalelectronics.ru>
Subject: Re: [PATCH 4/5] net: stmmac: Add glue layer for Loongson-1 SoC
On Sat, Aug 19, 2023 at 12:19 AM Serge Semin <fancer.lancer@...il.com> wrote:
>
> On Fri, Aug 18, 2023 at 08:37:27PM +0800, Keguang Zhang wrote:
> > On Wed, Aug 16, 2023 at 9:30 PM Serge Semin <fancer.lancer@...il.com> wrote:
> > >
> > > On Sat, Aug 12, 2023 at 11:11:34PM +0800, Keguang Zhang wrote:
> > > > This glue driver is created based on the arch-code
> > > > implemented earlier with the platform-specific settings.
> > > >
> > > > Use syscon for SYSCON register access.
> > > >
> > > > Partialy based on the previous work by Serge Semin.
> > > >
> > > > Signed-off-by: Keguang Zhang <keguang.zhang@...il.com>
> > > > ---
> > > > drivers/net/ethernet/stmicro/stmmac/Kconfig | 11 +
> > > > drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
> > > > .../ethernet/stmicro/stmmac/dwmac-loongson1.c | 257 ++++++++++++++++++
> > > > 3 files changed, 269 insertions(+)
> > > > create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-loongson1.c
> > > >
> > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> > > > index 06c6871f8788..a2b9e289aa36 100644
> > > > --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> > > > +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> > > > @@ -239,6 +239,17 @@ config DWMAC_INTEL_PLAT
> > > > the stmmac device driver. This driver is used for the Intel Keem Bay
> > > > SoC.
> > > >
> > > > +config DWMAC_LOONGSON1
> > > > + tristate "Loongson1 GMAC support"
> > > > + default MACH_LOONGSON32
> > > > + depends on OF && (MACH_LOONGSON32 || COMPILE_TEST)
> > > > + help
> > > > + Support for ethernet controller on Loongson1 SoC.
> > > > +
> > > > + This selects Loongson1 SoC glue layer support for the stmmac
> > > > + device driver. This driver is used for Loongson1-based boards
> > > > + like Loongson LS1B/LS1C.
> > > > +
> > > > config DWMAC_TEGRA
> > > > tristate "NVIDIA Tegra MGBE support"
> > > > depends on ARCH_TEGRA || COMPILE_TEST
> > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
> > > > index 5b57aee19267..80e598bd4255 100644
> > > > --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
> > > > +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
> > > > @@ -29,6 +29,7 @@ obj-$(CONFIG_DWMAC_SUNXI) += dwmac-sunxi.o
> > > > obj-$(CONFIG_DWMAC_SUN8I) += dwmac-sun8i.o
> > > > obj-$(CONFIG_DWMAC_DWC_QOS_ETH) += dwmac-dwc-qos-eth.o
> > > > obj-$(CONFIG_DWMAC_INTEL_PLAT) += dwmac-intel-plat.o
> > > > +obj-$(CONFIG_DWMAC_LOONGSON1) += dwmac-loongson1.o
> > > > obj-$(CONFIG_DWMAC_GENERIC) += dwmac-generic.o
> > > > obj-$(CONFIG_DWMAC_IMX8) += dwmac-imx.o
> > > > obj-$(CONFIG_DWMAC_TEGRA) += dwmac-tegra.o
> > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson1.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson1.c
> > > > new file mode 100644
> > > > index 000000000000..368d6cd2cb78
> > > > --- /dev/null
> > > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson1.c
> > > > @@ -0,0 +1,257 @@
> > > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > > +/*
> > > > + * Loongson-1 DWMAC glue layer
> > > > + *
> > > > + * Copyright (C) 2011-2023 Keguang Zhang <keguang.zhang@...il.com>
> > > > + */
> > > > +
> > > > +#include <linux/mfd/syscon.h>
> > > > +#include <linux/module.h>
> > > > +#include <linux/phy.h>
> > > > +#include <linux/platform_device.h>
> > > > +#include <linux/regmap.h>
> > > > +
> > > > +#include "stmmac.h"
> > > > +#include "stmmac_platform.h"
> > > > +
> > > > +/* Loongson-1 SYSCON Registers */
> > > > +#define LS1X_SYSCON0 (0x0)
> > > > +#define LS1X_SYSCON1 (0x4)
> > > > +
> > > > +struct ls1x_dwmac_syscon {
> > > > + const struct reg_field *reg_fields;
> > > > + unsigned int nr_reg_fields;
> > > > + int (*syscon_init)(struct plat_stmmacenet_data *plat);
> > > > +};
> > > > +
> > > > +struct ls1x_dwmac {
> > > > + struct device *dev;
> > > > + struct plat_stmmacenet_data *plat_dat;
> > > > + const struct ls1x_dwmac_syscon *syscon;
> > > > + struct regmap *regmap;
> > > > + struct regmap_field *regmap_fields[];
> > > > +};
> > > > +
> > > > +enum ls1b_dwmac_syscon_regfield {
> > > > + GMAC1_USE_UART1,
> > > > + GMAC1_USE_UART0,
> > > > + GMAC1_SHUT,
> > > > + GMAC0_SHUT,
> > > > + GMAC1_USE_TXCLK,
> > > > + GMAC0_USE_TXCLK,
> > > > + GMAC1_USE_PWM23,
> > > > + GMAC0_USE_PWM01,
> > > > +};
> > > > +
> > > > +enum ls1c_dwmac_syscon_regfield {
> > > > + GMAC_SHUT,
> > > > + PHY_INTF_SELI,
> > > > +};
> > > > +
> > > > +const struct reg_field ls1b_dwmac_syscon_regfields[] = {
> > > > + [GMAC1_USE_UART1] = REG_FIELD(LS1X_SYSCON0, 4, 4),
> > > > + [GMAC1_USE_UART0] = REG_FIELD(LS1X_SYSCON0, 3, 3),
> > > > + [GMAC1_SHUT] = REG_FIELD(LS1X_SYSCON1, 13, 13),
> > > > + [GMAC0_SHUT] = REG_FIELD(LS1X_SYSCON1, 12, 12),
> > > > + [GMAC1_USE_TXCLK] = REG_FIELD(LS1X_SYSCON1, 3, 3),
> > > > + [GMAC0_USE_TXCLK] = REG_FIELD(LS1X_SYSCON1, 2, 2),
> > > > + [GMAC1_USE_PWM23] = REG_FIELD(LS1X_SYSCON1, 1, 1),
> > > > + [GMAC0_USE_PWM01] = REG_FIELD(LS1X_SYSCON1, 0, 0)
> > > > +};
> > > > +
> > > > +const struct reg_field ls1c_dwmac_syscon_regfields[] = {
> > > > + [GMAC_SHUT] = REG_FIELD(LS1X_SYSCON0, 6, 6),
> > > > + [PHY_INTF_SELI] = REG_FIELD(LS1X_SYSCON1, 28, 30)
> > > > +};
> > >
> > > Emm, using regmap fields looks so over-complicated in this case seeing
> > > you only need to set/clear several bits in the syscon. What about
> > > defining macros with the particular flag as it's already done in the
> > > "asm/mach-loongson32/regs-mux.h" file and using regmap_update_bits()?
> > >
>
> > To use regmap_update_bits(), I have to store and pass reg_offset and
> > mask, which is similar to the definition of regmap fields.
>
> Em, not really. And what offset are you talking about? Anyway you
> don't need one. Moreover you'll be able to reduce the number of IOs:
>
> +#define GMAC1_USE_UART1 BIT(4)
> +#define GMAC1_USE_UART0 BIT(3)
> ...
> +#define GMAC1_SHUT BIT(13)
> ...
> +#define GMAC1_USE_TXCLK BIT(3)
> +#define GMAC1_USE_PWM23 BIT(1)
>
> +static int ls1b_dwmac_syscon_init(struct plat_stmmacenet_data *plat)
> +{
> + struct ls1x_dwmac *dwmac = plat->bsp_priv;
> + struct regmap *syscon = dwmac->regmap;
> +
> + if (plat->bus_id) {
> + regmap_update_bits(syscon, LS1X_SYSCON0,
> + GMAC1_USE_UART1 | GMAC1_USE_UART0,
> + GMAC1_USE_UART1 | GMAC1_USE_UART0);
> +
> + switch (plat->phy_interface) {
> + case PHY_INTERFACE_MODE_RGMII:
> + regmap_update_bits(syscon, LS1X_SYSCON1,
> + GMAC1_USE_TXCLK | GMAC1_USE_TXCLK, 0);
> + break;
> + case PHY_INTERFACE_MODE_MII:
> + regmap_update_bits(syscon, LS1X_SYSCON1,
> + GMAC1_USE_TXCLK | GMAC1_USE_TXCLK
> + GMAC1_USE_TXCLK | GMAC1_USE_TXCLK);
> + break;
> + default:
> + dev_err(dwmac->dev, "Unsupported PHY mode %u\n",
> + plat->phy_interface);
> + return -EOPNOTSUPP;
> + }
> +
> + regmap_field_write(syscon, LS1X_SYSCON1, GMAC1_SHUT, 0);
> + } //...
> +
> + return 0;
> +}
>
> This doesn't look in anyway less readable then your implementation
> but in fact simpler.
>
> > In addition, the regmap fields are very clear and leave the trouble to
> > the internal implementation.
>
> In this case it brings much more troubles and no clarity. You need to create
> an additional mainly redundant abstraction, waste memory for it,
> define additional const arrays. Using it won't improve the code
> readability seeing you need to set/clear a few flags only. So all of
> the troubles for nothing. See the code above. It's simple and clear.
> Just several regmap_update_bits()..
>
OK. I will use regmap instead of regmap fields.
> BTW why have you chosen to define syscon instead of creating a pinctrl
> driver? What if Loongson1 is embedded into a platform with, for
> instance, UART0 and UART1 utilized instead of the GMAC1?
>
As you can see, the two registers contains miscellaneous settings.
Besides ‘USE’ bits, there are ‘RESET‘ bits, 'EN' bits, 'SHUT' bits, ...
So they are not pinctrl registers.
Actually, there is a dedicated pin controller which controls the
multiplexing of pads.
> >
> > > > +
> > >
> > > > +static int ls1b_dwmac_syscon_init(struct plat_stmmacenet_data *plat)
> > > > +{
> > >
> > > As I already told you this part is better to be called from the
> > > plat_stmmacenet_data.fix_mac_speed() because PHY interface mode can
> > > differ from one interface open cycle to another as per the phylink
> > > design.
> > >
> > I have considered .fix_mac_speed(), which will be called every time
> > the link is up, and passes the current speed.
> > However, the PHY interface mode is determined by the hardware design -
> > the schematic.
> > In other words, once the schematic is done, the PHY interface mode is fixed.
> > Therefore, PHY interface mode should be configured one time at the
> > initialization.
> > And the plat_stmmacenet_data.init() is the proper place to do this.
>
> Ok. If no actual clock change is needed then indeed init() will be the
> proper place.
>
> >
> > > > + struct ls1x_dwmac *dwmac = plat->bsp_priv;
> > > > + struct regmap_field **regmap_fields = dwmac->regmap_fields;
> > > > +
> > >
> > > > + if (plat->bus_id) {
> > >
> > > Using bus_id doesn't look correct to determine the CSRs responsible
> > > for the interface mode selection because it's calculated based on the
> > > DT ethernet-alias which doesn't guarantee to have a particular device
> > > assigned with the alias. Alias node can be absent after all. What
> > > could be better in this case is for instance to use the regs physical
> > > address. Any better idea?
> > >
>
> > The purpose of alias is to bind the a particular device with a
> > particular alias even some aliases are absent.
> > Because of_alias_get_id() gets the alias id.
> > For example, LS1B has two GMAC controllers, gmac0 and gmac1.
> > I have tried the Ethernet with only one alias as follows.
> > aliases {
> > ethernet1 = &gmac1;
> > };
> > In this case, plat->bus_id is still 1.
> > And both gmac0 and gmac1 work.
>
> If no alias specified? If both aliases a non zero? If the IDs are
> confused? If any of these is true you are in trouble. Your code
> shouldn't rely on the aliases in this case. You need to come up with a
> way to certainly distinguish one MAC from another. A physical base
> address is one possible option.
>
I see.
But It seems unusual to determine device IDs by physical base address.
What about adding a new property? such as loongson,dwmac-id
> Note the /alias node is an informational node. It doesn't describe
> devices. Just recent Krzysztof comment in a similar situation:
> https://lore.kernel.org/netdev/20230814112539.70453-1-sriranjani.p@samsung.com/T/#m3972e40bd2fa323a3bdb2fbf07bde47ba6752439
>
> Aliases are normally used by OS to for instance fix the device
> enumeration (see SPI, I2C, I3C, MTD, MMC, RTC, TTY/Serial, Watchdog,
> MDIO-GPIO, etc) - pre-define the device ID from the kernel or OS point
> of view. In your case the IDs can't be changed. GMAC0 must be assigned
> with ID0 and GMAC1 must be assigned with non-zero. Doing otherwise
> will be break the interfaces functionality which isn't acceptable.
>
> >
> > > > + regmap_field_write(regmap_fields[GMAC1_USE_UART1], 1);
> > > > + regmap_field_write(regmap_fields[GMAC1_USE_UART0], 1);
> > > > +
> > > > + switch (plat->phy_interface) {
> > > > + case PHY_INTERFACE_MODE_RGMII:
> > > > + regmap_field_write(regmap_fields[GMAC1_USE_TXCLK], 0);
> > > > + regmap_field_write(regmap_fields[GMAC1_USE_PWM23], 0);
> > > > + break;
> > > > + case PHY_INTERFACE_MODE_MII:
> > > > + regmap_field_write(regmap_fields[GMAC1_USE_TXCLK], 1);
> > > > + regmap_field_write(regmap_fields[GMAC1_USE_PWM23], 1);
> > > > + break;
> > > > + default:
> > > > + dev_err(dwmac->dev, "Unsupported PHY mode %u\n",
> > > > + plat->phy_interface);
> > > > + return -EOPNOTSUPP;
> > > > + }
> > > > +
> > > > + regmap_field_write(regmap_fields[GMAC1_SHUT], 0);
> > > > + } else {
> > > > + switch (plat->phy_interface) {
> > > > + case PHY_INTERFACE_MODE_RGMII:
> > > > + regmap_field_write(regmap_fields[GMAC0_USE_TXCLK], 0);
> > > > + regmap_field_write(regmap_fields[GMAC0_USE_PWM01], 0);
> > > > + break;
> > > > + case PHY_INTERFACE_MODE_MII:
> > > > + regmap_field_write(regmap_fields[GMAC0_USE_TXCLK], 1);
> > > > + regmap_field_write(regmap_fields[GMAC0_USE_PWM01], 1);
> > > > + break;
> > > > + default:
> > > > + dev_err(dwmac->dev, "Unsupported PHY mode %u\n",
> > > > + plat->phy_interface);
> > > > + return -EOPNOTSUPP;
> > > > + }
> > > > +
> > > > + regmap_field_write(regmap_fields[GMAC0_SHUT], 0);
> > > > + }
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int ls1c_dwmac_syscon_init(struct plat_stmmacenet_data *plat)
> > > > +{
> > > > + struct ls1x_dwmac *dwmac = plat->bsp_priv;
> > > > + struct regmap_field **regmap_fields = dwmac->regmap_fields;
> > > > +
> > > > + if (plat->phy_interface == PHY_INTERFACE_MODE_RMII) {
> > > > + regmap_field_write(regmap_fields[PHY_INTF_SELI], 0x4);
> > > > + } else {
> > > > + dev_err(dwmac->dev, "Unsupported PHY-mode %u\n",
> > > > + plat->phy_interface);
> > > > + return -EOPNOTSUPP;
> > > > + }
> > > > +
> > > > + regmap_field_write(regmap_fields[GMAC_SHUT], 0);
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static const struct ls1x_dwmac_syscon ls1b_dwmac_syscon = {
> > > > + .reg_fields = ls1b_dwmac_syscon_regfields,
> > > > + .nr_reg_fields = ARRAY_SIZE(ls1b_dwmac_syscon_regfields),
> > > > + .syscon_init = ls1b_dwmac_syscon_init,
> > > > +};
> > > > +
> > > > +static const struct ls1x_dwmac_syscon ls1c_dwmac_syscon = {
> > > > + .reg_fields = ls1c_dwmac_syscon_regfields,
> > > > + .nr_reg_fields = ARRAY_SIZE(ls1c_dwmac_syscon_regfields),
> > > > + .syscon_init = ls1c_dwmac_syscon_init,
> > > > +};
> > > > +
> > > > +static int ls1x_dwmac_init(struct platform_device *pdev, void *priv)
> > > > +{
> > > > + struct ls1x_dwmac *dwmac = priv;
> > > > + int ret;
> > > > +
> > >
> > > > + ret = devm_regmap_field_bulk_alloc(dwmac->dev, dwmac->regmap,
> > > > + dwmac->regmap_fields,
> > > > + dwmac->syscon->reg_fields,
> > > > + dwmac->syscon->nr_reg_fields);
> > >
> > > Please see my first comment about this.
> > >
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + if (dwmac->syscon->syscon_init) {
> > > > + ret = dwmac->syscon->syscon_init(dwmac->plat_dat);
> > > > + if (ret)
> > > > + return ret;
> > > > + }
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static const struct of_device_id ls1x_dwmac_syscon_match[] = {
> > > > + { .compatible = "loongson,ls1b-syscon", .data = &ls1b_dwmac_syscon },
> > > > + { .compatible = "loongson,ls1c-syscon", .data = &ls1c_dwmac_syscon },
> > > > + { }
> > > > +};
> > > > +
> > > > +static int ls1x_dwmac_probe(struct platform_device *pdev)
> > > > +{
> > > > + struct plat_stmmacenet_data *plat_dat;
> > > > + struct stmmac_resources stmmac_res;
> > > > + struct device_node *syscon_np;
> > > > + const struct of_device_id *match;
> > > > + struct regmap *regmap;
> > > > + struct ls1x_dwmac *dwmac;
> > > > + const struct ls1x_dwmac_syscon *syscon;
> > > > + size_t size;
> > > > + int ret;
> > > > +
> > > > + ret = stmmac_get_platform_resources(pdev, &stmmac_res);
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > >
> > > > + /* Probe syscon */
> > > > + syscon_np = of_parse_phandle(pdev->dev.of_node, "syscon", 0);
> > >
> > > it's vendor-specific property so it is supposed to have a
> > > vendor-specific prefix and possibly ls1-specific name.
> > >
> > This has been fixed in v2.
> > Could you please review v2?
> > Thanks!
> >
> > > > + if (!syscon_np)
> > > > + return -ENODEV;
> > > > +
> > > > + match = of_match_node(ls1x_dwmac_syscon_match, syscon_np);
> > > > + if (!match) {
> > > > + of_node_put(syscon_np);
> > > > + return -EINVAL;
> > > > + }
> > > > + syscon = (const struct ls1x_dwmac_syscon *)match->data;
Please note that of_match_node() is used for syscon matching.
> > > > +
> > > > + regmap = syscon_node_to_regmap(syscon_np);
> > > > + of_node_put(syscon_np);
> > > > + if (IS_ERR(regmap)) {
> > > > + ret = PTR_ERR(regmap);
> > > > + dev_err(&pdev->dev, "Unable to map syscon: %d\n", ret);
> > > > + return ret;
> > > > + }
> > >
> > > or you can use syscon_regmap_lookup_by_phandle(). Using
> > > of_match_node() doesn't seem necessary since it's unlikely to have
> > > moee than one system controller available on the LS1b or LS1c chips.
> > >
>
> > I planned to use syscon_regmap_lookup_by_phandle().
> > Thus the compatible
> > "loongson,ls1b-dwmac-syscon"/"loongson,ls1c-dwmac-syscon" would become
> > useless.
> > I'm not sure about this.
>
> The compatible strings should be left despite of the
> syscon_regmap_lookup_by_phandle() usage. But again "dwmac" suffix is
> redundant. Based on the CSRs definition in regs-mux.h, selecting
> (G)MAC pins mode is only a small part of the Loongson1 SoC system
> controllers functionality.
> "loongson,ls1b-syscon"/"loongson,ls1c-syscon" looks more appropriate.
>
That's what I did in PATCH 2/5.
I've just explained this to Krzysztof.
And will change back to "loongson,ls1b-syscon"/"loongson,ls1c-syscon"
in next version.
In addition, syscon_regmap_lookup_by_phandle() returns regmap pointer directly.
Then, there wil be no way to do syscon matching without its device_node.
How will I know whether the syscon is loongson,ls1b-syscon or
loongson,ls1c-syscon?
Thanks for your review!
> -Serge(y)
>
> >
> > > > +
> > > > + size = syscon->nr_reg_fields * sizeof(struct regmap_field *);
> > > > + dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac) + size, GFP_KERNEL);
> > > > + if (!dwmac)
> > > > + return -ENOMEM;
> > > > +
> > > > + plat_dat = stmmac_probe_config_dt(pdev, stmmac_res.mac);
> > > > + if (IS_ERR(plat_dat)) {
> > > > + dev_err(&pdev->dev, "dt configuration failed\n");
> > > > + return PTR_ERR(plat_dat);
> > > > + }
> > > > +
> > > > + plat_dat->bsp_priv = dwmac;
> > > > + plat_dat->init = ls1x_dwmac_init;
> > > > + dwmac->dev = &pdev->dev;
> > > > + dwmac->plat_dat = plat_dat;
> > > > + dwmac->syscon = syscon;
> > > > + dwmac->regmap = regmap;
> > > > +
> > > > + ret = stmmac_pltfr_probe(pdev, plat_dat, &stmmac_res);
> > > > + if (ret)
> > > > + goto err_remove_config_dt;
> > > > +
> > > > + return 0;
> > > > +
> > > > +err_remove_config_dt:
> > >
> > > > + if (pdev->dev.of_node)
> > >
> > > Is this conditional statement necessary here?
> > >
> > You're right.
> > Will remove this condition in next version.
> > Thanks!
> >
> > > -Serge
> > >
> > > > + stmmac_remove_config_dt(pdev, plat_dat);
> > > > +
> > > > + return ret;
> > > > +}
> > > > +
> > > > +static const struct of_device_id ls1x_dwmac_match[] = {
> > > > + { .compatible = "loongson,ls1b-dwmac" },
> > > > + { .compatible = "loongson,ls1c-dwmac" },
> > > > + { }
> > > > +};
> > > > +MODULE_DEVICE_TABLE(of, ls1x_dwmac_match);
> > > > +
> > > > +static struct platform_driver ls1x_dwmac_driver = {
> > > > + .probe = ls1x_dwmac_probe,
> > > > + .remove_new = stmmac_pltfr_remove,
> > > > + .driver = {
> > > > + .name = "loongson1-dwmac",
> > > > + .of_match_table = ls1x_dwmac_match,
> > > > + },
> > > > +};
> > > > +module_platform_driver(ls1x_dwmac_driver);
> > > > +
> > > > +MODULE_AUTHOR("Keguang Zhang <keguang.zhang@...il.com>");
> > > > +MODULE_DESCRIPTION("Loongson1 DWMAC glue layer");
> > > > +MODULE_LICENSE("GPL");
> > > > --
> > > > 2.39.2
> > > >
> >
> >
> >
> > --
> > Best regards,
> >
> > Keguang Zhang
--
Best regards,
Keguang Zhang
Powered by blists - more mailing lists