Re: [PATCH v4 1/2] net: cortina_ni: Add eth support for Cortina Access CAxxxx SoCs
On Wed, Jun 3, 2020 at 11:05 AM Alex Nemirovsky
wrote:
>
> From: Aaron Tseng
>
> Add Cortina Access Ethernet device driver for CA SoCs.
> This driver supports only the DM_ETH network model.
>
> Signed-off-by: Aaron Tseng
> Signed-off-by: Alex Nemirovsky
>
> CC: Joe Hershberger
> CC: Abbie Chang
> CC: Tom Rini
>
> ---
>
> Changes in v4: None
> Changes in v3:
> - Changed commit comment to state that only DM model is supported
> - Removed blank line at end of C file
>
> Changes in v2:
> - Remove legacy mode support
> - Add support for additional SoC variants
> - Remove unused variables
>
> MAINTAINERS |4 +
> drivers/net/Kconfig |7 +
> drivers/net/Makefile |1 +
> drivers/net/cortina_ni.c | 1909
> ++
> drivers/net/cortina_ni.h | 592 ++
> 5 files changed, 2513 insertions(+)
> create mode 100644 drivers/net/cortina_ni.c
> create mode 100644 drivers/net/cortina_ni.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8add9d4..1b166d2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -181,6 +181,8 @@ F: drivers/gpio/cortina_gpio.c
> F: drivers/watchdog/cortina_wdt.c
> F: drivers/serial/serial_cortina.c
> F: drivers/mmc/ca_dw_mmc.c
> +F: drivers/net/cortina_ni.c
> +F: drivers/net/cortina_ni.h
>
> ARM/CZ.NIC TURRIS MOX SUPPORT
> M: Marek Behun
> @@ -732,6 +734,8 @@ F: drivers/gpio/cortina_gpio.c
> F: drivers/watchdog/cortina_wdt.c
> F: drivers/serial/serial_cortina.c
> F: drivers/mmc/ca_dw_mmc.c
> +F: drivers/net/cortina_ni.c
> +F: drivers/net/cortina_ni.h
>
> MIPS MSCC
> M: Gregory CLEMENT
> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index f7855c9..45e0480 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -149,6 +149,13 @@ config BCMGENET
> help
> This driver supports the BCMGENET Ethernet MAC.
>
> +config CORTINA_NI_ENET
> + bool "Cortina-Access Ethernet driver"
> + depends on DM_ETH && CORTINA_PLATFORM
> + help
> + The driver supports the Cortina-Access Ethernet MAC for
> + all supported CA SoCs
> +
> config DWC_ETH_QOS
> bool "Synopsys DWC Ethernet QOS device support"
> depends on DM_ETH
> diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> index 383ed1c..1d6ec4f 100644
> --- a/drivers/net/Makefile
> +++ b/drivers/net/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_DRIVER_AX88180) += ax88180.o
> obj-$(CONFIG_BCM_SF2_ETH) += bcm-sf2-eth.o
> obj-$(CONFIG_BCM_SF2_ETH_GMAC) += bcm-sf2-eth-gmac.o
> obj-$(CONFIG_CALXEDA_XGMAC) += calxedaxgmac.o
> +obj-$(CONFIG_CORTINA_NI_ENET) += cortina_ni.o
> obj-$(CONFIG_CS8900) += cs8900.o
> obj-$(CONFIG_TULIP) += dc2114x.o
> obj-$(CONFIG_ETH_DESIGNWARE) += designware.o
> diff --git a/drivers/net/cortina_ni.c b/drivers/net/cortina_ni.c
> new file mode 100644
> index 000..d5bbf3e
> --- /dev/null
> +++ b/drivers/net/cortina_ni.c
> @@ -0,0 +1,1909 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/*
> + * Copyright (C) 2020 Cortina Access Inc.
> + * Author: Aaron Tseng
> + *
> + * Ethernet MAC Driver for all supported CA SoCs
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "cortina_ni.h"
> +
> +static u32 reg_value;
> +
> +/* port 0-3 are individual port connect to PHY directly */
> +/* port 4-7 are LAN ports connected to QSGMII PHY */
> +int active_port = NI_PORT_5; /* Physical port 5 */
> +u32 ge_port_phy_addr; /* PHY address connected to active port */
> +int auto_scan_active_port;
> +
> +#define HEADER_A_SIZE 8
> +
> +/*define CORTINA_NI_DBG if individual rx,tx,init needs to be called */
> +#if CORTINA_NI_DBG
> +static struct udevice *dbg_dev;
> +#endif
> +static struct udevice *curr_dev;
> +
> +#if defined(CONFIG_TARGET_SATURN_ASIC)
> +#define CA_REG_READ(off)readl((u64)KSEG1_ATU_XLAT(off))
> +#define CA_REG_WRITE(data, off) writel(data, (u64)KSEG1_ATU_XLAT(off))
> +#else
> +#define CA_REG_READ(off)readl((u64)off)
> +#define CA_REG_WRITE(data, off) writel(data, (u64)off)
> +#endif
> +
> +int cortina_ni_recv(struct udevice *netdev);
> +static int ca_ni_ofdata_to_platdata(struct udevice *dev);
> +
> +static u32 *RDWRPTR_ADVANCE_ONE(u32 *x, unsigned long base, unsigned long
> max)
Please don't use upper case function names.
> +{
> + if (x + 1 >= (u32 *)max)
> + return (u32 *)base;
> + else
> + return (x + 1);
> +}
> +
> +static void ni_setup_mac_addr(void)
> +{
> + unsigned char mac[6];
> +
> + union NI_HV_GLB_MAC_ADDR_CFG0_t mac_addr_cfg0;
> + union NI_HV_GLB_MAC_ADDR_CFG1_t mac_addr_cfg1;
> + union NI_HV_PT_PORT_STATIC_CFG_tport_static_cfg;
> + union NI_HV_XRAM_CPUXRAM_CFG_t cpuxram_cfg;
> + struct cortina_ni_priv *priv = dev_get_priv(curr_dev);
> +
> + /* parsing ethaddr and set to NI
Re: [PATCH v4 1/2] net: cortina_ni: Add eth support for Cortina Access CAxxxx SoCs
Hi Tom, Thanks for your feedback. > On Jun 3, 2020, at 3:03 PM, Tom Rini wrote: > > On Wed, Jun 03, 2020 at 09:10:46PM +, Alex Nemirovsky wrote: >> Hi Tom >> >>> On Jun 3, 2020, at 1:59 PM, Tom Rini wrote: >>> >>> On Wed, Jun 03, 2020 at 07:03:09PM +, Alex Nemirovsky wrote: Hi Tom, > On Jun 3, 2020, at 8:03 AM, Tom Rini wrote: > > On Wed, Jun 03, 2020 at 01:05:18AM -0700, Alex Nemirovsky wrote: >> From: Aaron Tseng >> >> Add Cortina Access Ethernet device driver for CA SoCs. >> This driver supports only the DM_ETH network model. >> >> Signed-off-by: Aaron Tseng >> Signed-off-by: Alex Nemirovsky >> >> CC: Joe Hershberger >> CC: Abbie Chang >> CC: Tom Rini >> >> --- >> >> Changes in v4: None >> Changes in v3: >> - Changed commit comment to state that only DM model is supported >> - Removed blank line at end of C file >> >> Changes in v2: >> - Remove legacy mode support >> - Add support for additional SoC variants >> - Remove unused variables >> >> MAINTAINERS |4 + >> drivers/net/Kconfig |7 + >> drivers/net/Makefile |1 + >> drivers/net/cortina_ni.c | 1909 >> ++ >> drivers/net/cortina_ni.h | 592 ++ >> 5 files changed, 2513 insertions(+) >> create mode 100644 drivers/net/cortina_ni.c >> create mode 100644 drivers/net/cortina_ni.h > > So, is there a similar driver in upstream Linux? We don’t have ANY Linux drivers upstream. >>> >>> Ah right. >>> Let me see if I can parse the comments below into action items for us. Let me know if I got it right or missed something. > This driver doesn't > quite fit right. > At an "easy" level, there's still the customer macro > around debug statements and not using pr_debug(), find and convert all debug statement to using U-Boot specific pr_debug(). >>> >>> pr_debug and other functions, and a handful of other print functions are >>> in and will both mean you don't need to wrap debug >>> statements but also leverage the logging functionality correctly so that >>> we can also discard messages to save space but also still easily bring >>> in useful information when a developer has a problem they're tracing. >>> > and contains a whole > lot of debug code. remove development debug code which is no longer used. >>> >>> Yes, and probably then do some clean-up of the functions once that's >>> removed, if that then makes sense. >>> > There's also code for a saturn platform, is that > being upstreamed? Yes, it will be upstreamed to u-boot.The approach is to first focus on getting all our u-boot drivers upstream for the presidio SoC engineering board. Most of the drivers are designed to work with other SoC platforms as those SoC reuse HW logic for our peripherals. >>> >>> OK, so lets leave the stuff for other platforms out for now. If there's >>> abstractions that are needed for them, you can note that's why there's a >>> hook when there is a hook. But that's probably still better handled >>> when introducing / posting the platform that needs them for further >>> context. >>> > There's a ton of union usage which is also uncommon. Is there an action item here? >>> >>> Well, why is it written the way it's written? >> >> The SW team didn’t create these structures by hand. They are autogenerated >> my our HW design tools and adopted by the SW team. >> To be clear, I am not trying to defend the use of this auto generated >> structures, just explaining its origin. >> Obviously it will take significant effort for us to recreate these >> structures in the desired form. >> >> Therefore, it must be asked. Is it required to be recreated in the >> structure form that you request and C code modified >> accordingly to be accepted into the u-boot upstream tree going forward? > > Yes, and work with the HW design tool folks to generate output that's > more acceptable to various open source projects. You aren't the first > team to have to push for this, I can tell you from experience. > > But since it's generated output, it can be transformed too via sed, etc. > Thanks! > > -- > Tom
Re: [PATCH v4 1/2] net: cortina_ni: Add eth support for Cortina Access CAxxxx SoCs
On Wed, Jun 03, 2020 at 09:10:46PM +, Alex Nemirovsky wrote: > Hi Tom > > > On Jun 3, 2020, at 1:59 PM, Tom Rini wrote: > > > > On Wed, Jun 03, 2020 at 07:03:09PM +, Alex Nemirovsky wrote: > >> Hi Tom, > >> > >>> On Jun 3, 2020, at 8:03 AM, Tom Rini wrote: > >>> > >>> On Wed, Jun 03, 2020 at 01:05:18AM -0700, Alex Nemirovsky wrote: > From: Aaron Tseng > > Add Cortina Access Ethernet device driver for CA SoCs. > This driver supports only the DM_ETH network model. > > Signed-off-by: Aaron Tseng > Signed-off-by: Alex Nemirovsky > > CC: Joe Hershberger > CC: Abbie Chang > CC: Tom Rini > > --- > > Changes in v4: None > Changes in v3: > - Changed commit comment to state that only DM model is supported > - Removed blank line at end of C file > > Changes in v2: > - Remove legacy mode support > - Add support for additional SoC variants > - Remove unused variables > > MAINTAINERS |4 + > drivers/net/Kconfig |7 + > drivers/net/Makefile |1 + > drivers/net/cortina_ni.c | 1909 > ++ > drivers/net/cortina_ni.h | 592 ++ > 5 files changed, 2513 insertions(+) > create mode 100644 drivers/net/cortina_ni.c > create mode 100644 drivers/net/cortina_ni.h > >>> > >>> So, is there a similar driver in upstream Linux? > >> > >> We don’t have ANY Linux drivers upstream. > > > > Ah right. > > > >> Let me see if I can parse the comments below into action items for us. > >> Let me know if I got it right or missed something. > >> > >>> This driver doesn't > >>> quite fit right. > >> > >> > >>> At an "easy" level, there's still the customer macro > >>> around debug statements and not using pr_debug(), > >> > >> find and convert all debug statement to using U-Boot specific pr_debug(). > > > > pr_debug and other functions, and a handful of other print functions are > > in and will both mean you don't need to wrap debug > > statements but also leverage the logging functionality correctly so that > > we can also discard messages to save space but also still easily bring > > in useful information when a developer has a problem they're tracing. > > > >>> and contains a whole > >>> lot of debug code. > >> > >> remove development debug code which is no longer used. > > > > Yes, and probably then do some clean-up of the functions once that's > > removed, if that then makes sense. > > > >>> There's also code for a saturn platform, is that > >>> being upstreamed? > >> Yes, it will be upstreamed to u-boot.The approach is to first focus on > >> getting > >> all our u-boot drivers upstream for the presidio SoC engineering board. > >> Most of the drivers are designed to work with other SoC platforms as those > >> SoC reuse HW logic for our peripherals. > > > > OK, so lets leave the stuff for other platforms out for now. If there's > > abstractions that are needed for them, you can note that's why there's a > > hook when there is a hook. But that's probably still better handled > > when introducing / posting the platform that needs them for further > > context. > > > >>> There's a ton of union usage which is also uncommon. > >> Is there an action item here? > > > > Well, why is it written the way it's written? > > The SW team didn’t create these structures by hand. They are autogenerated > my our HW design tools and adopted by the SW team. > To be clear, I am not trying to defend the use of this auto generated > structures, just explaining its origin. > Obviously it will take significant effort for us to recreate these structures > in the desired form. > > Therefore, it must be asked. Is it required to be recreated in the structure > form that you request and C code modified > accordingly to be accepted into the u-boot upstream tree going forward? Yes, and work with the HW design tool folks to generate output that's more acceptable to various open source projects. You aren't the first team to have to push for this, I can tell you from experience. But since it's generated output, it can be transformed too via sed, etc. Thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v4 1/2] net: cortina_ni: Add eth support for Cortina Access CAxxxx SoCs
Hi Tom
> On Jun 3, 2020, at 1:59 PM, Tom Rini wrote:
>
> On Wed, Jun 03, 2020 at 07:03:09PM +, Alex Nemirovsky wrote:
>> Hi Tom,
>>
>>> On Jun 3, 2020, at 8:03 AM, Tom Rini wrote:
>>>
>>> On Wed, Jun 03, 2020 at 01:05:18AM -0700, Alex Nemirovsky wrote:
From: Aaron Tseng
Add Cortina Access Ethernet device driver for CA SoCs.
This driver supports only the DM_ETH network model.
Signed-off-by: Aaron Tseng
Signed-off-by: Alex Nemirovsky
CC: Joe Hershberger
CC: Abbie Chang
CC: Tom Rini
---
Changes in v4: None
Changes in v3:
- Changed commit comment to state that only DM model is supported
- Removed blank line at end of C file
Changes in v2:
- Remove legacy mode support
- Add support for additional SoC variants
- Remove unused variables
MAINTAINERS |4 +
drivers/net/Kconfig |7 +
drivers/net/Makefile |1 +
drivers/net/cortina_ni.c | 1909
++
drivers/net/cortina_ni.h | 592 ++
5 files changed, 2513 insertions(+)
create mode 100644 drivers/net/cortina_ni.c
create mode 100644 drivers/net/cortina_ni.h
>>>
>>> So, is there a similar driver in upstream Linux?
>>
>> We don’t have ANY Linux drivers upstream.
>
> Ah right.
>
>> Let me see if I can parse the comments below into action items for us.
>> Let me know if I got it right or missed something.
>>
>>> This driver doesn't
>>> quite fit right.
>>
>>
>>> At an "easy" level, there's still the customer macro
>>> around debug statements and not using pr_debug(),
>>
>> find and convert all debug statement to using U-Boot specific pr_debug().
>
> pr_debug and other functions, and a handful of other print functions are
> in and will both mean you don't need to wrap debug
> statements but also leverage the logging functionality correctly so that
> we can also discard messages to save space but also still easily bring
> in useful information when a developer has a problem they're tracing.
>
>>> and contains a whole
>>> lot of debug code.
>>
>> remove development debug code which is no longer used.
>
> Yes, and probably then do some clean-up of the functions once that's
> removed, if that then makes sense.
>
>>> There's also code for a saturn platform, is that
>>> being upstreamed?
>> Yes, it will be upstreamed to u-boot.The approach is to first focus on
>> getting
>> all our u-boot drivers upstream for the presidio SoC engineering board.
>> Most of the drivers are designed to work with other SoC platforms as those
>> SoC reuse HW logic for our peripherals.
>
> OK, so lets leave the stuff for other platforms out for now. If there's
> abstractions that are needed for them, you can note that's why there's a
> hook when there is a hook. But that's probably still better handled
> when introducing / posting the platform that needs them for further
> context.
>
>>> There's a ton of union usage which is also uncommon.
>> Is there an action item here?
>
> Well, why is it written the way it's written?
The SW team didn’t create these structures by hand. They are autogenerated
my our HW design tools and adopted by the SW team.
To be clear, I am not trying to defend the use of this auto generated
structures, just explaining its origin.
Obviously it will take significant effort for us to recreate these structures
in the desired form.
Therefore, it must be asked. Is it required to be recreated in the structure
form that you request and C code modified
accordingly to be accepted into the u-boot upstream tree going forward?
> It doesn't follow the
> style of other network drivers and so yes, it needs to be written in the
> same manner as other drivers. So for example:
> +union NI_HV_PT_PORT_STATIC_CFG_t {
> + struct {
> + unsigned int int_cfg : 4; /* bits 3:0 */
> + unsigned int phy_mode : 1; /* bits 4:4 */
> + unsigned int rmii_clksrc : 1; /* bits 5:5 */
> + unsigned int inv_clk_in : 1; /* bits 6:6 */
> + unsigned int inv_clk_out : 1; /* bits 7:7 */
> + unsigned int inv_rxclk_out: 1; /* bits 8:8 */
> + unsigned int tx_use_gefifo: 1; /* bits 9:9 */
> + unsigned int smii_tx_stat : 1; /* bits 10:10 */
> + unsigned int crs_polarity : 1; /* bits 11:11 */
> + unsigned int lpbk_mode: 2; /* bits 13:12 */
> + unsigned int gmii_like_half_duplex_en : 1; /* bits 14:14 */
> + unsigned int sup_tx_to_rx_lpbk_data : 1; /* bits 15:15 */
> + unsigned int rsrvd1 : 8;
> + unsigned int mac_addr6: 8; /* bits 31:24 */
> + } bf;
> + unsigned int wrd;
> +};
>
> But in other drivers
Re: [PATCH v4 1/2] net: cortina_ni: Add eth support for Cortina Access CAxxxx SoCs
On Wed, Jun 03, 2020 at 07:03:09PM +, Alex Nemirovsky wrote:
> Hi Tom,
>
> > On Jun 3, 2020, at 8:03 AM, Tom Rini wrote:
> >
> > On Wed, Jun 03, 2020 at 01:05:18AM -0700, Alex Nemirovsky wrote:
> >> From: Aaron Tseng
> >>
> >> Add Cortina Access Ethernet device driver for CA SoCs.
> >> This driver supports only the DM_ETH network model.
> >>
> >> Signed-off-by: Aaron Tseng
> >> Signed-off-by: Alex Nemirovsky
> >>
> >> CC: Joe Hershberger
> >> CC: Abbie Chang
> >> CC: Tom Rini
> >>
> >> ---
> >>
> >> Changes in v4: None
> >> Changes in v3:
> >> - Changed commit comment to state that only DM model is supported
> >> - Removed blank line at end of C file
> >>
> >> Changes in v2:
> >> - Remove legacy mode support
> >> - Add support for additional SoC variants
> >> - Remove unused variables
> >>
> >> MAINTAINERS |4 +
> >> drivers/net/Kconfig |7 +
> >> drivers/net/Makefile |1 +
> >> drivers/net/cortina_ni.c | 1909
> >> ++
> >> drivers/net/cortina_ni.h | 592 ++
> >> 5 files changed, 2513 insertions(+)
> >> create mode 100644 drivers/net/cortina_ni.c
> >> create mode 100644 drivers/net/cortina_ni.h
> >
> > So, is there a similar driver in upstream Linux?
>
> We don’t have ANY Linux drivers upstream.
Ah right.
> Let me see if I can parse the comments below into action items for us.
> Let me know if I got it right or missed something.
>
> > This driver doesn't
> > quite fit right.
>
>
> > At an "easy" level, there's still the customer macro
> > around debug statements and not using pr_debug(),
>
> find and convert all debug statement to using U-Boot specific pr_debug().
pr_debug and other functions, and a handful of other print functions are
in and will both mean you don't need to wrap debug
statements but also leverage the logging functionality correctly so that
we can also discard messages to save space but also still easily bring
in useful information when a developer has a problem they're tracing.
> > and contains a whole
> > lot of debug code.
>
> remove development debug code which is no longer used.
Yes, and probably then do some clean-up of the functions once that's
removed, if that then makes sense.
> > There's also code for a saturn platform, is that
> > being upstreamed?
> Yes, it will be upstreamed to u-boot.The approach is to first focus on getting
> all our u-boot drivers upstream for the presidio SoC engineering board.
> Most of the drivers are designed to work with other SoC platforms as those
> SoC reuse HW logic for our peripherals.
OK, so lets leave the stuff for other platforms out for now. If there's
abstractions that are needed for them, you can note that's why there's a
hook when there is a hook. But that's probably still better handled
when introducing / posting the platform that needs them for further
context.
> > There's a ton of union usage which is also uncommon.
> Is there an action item here?
Well, why is it written the way it's written? It doesn't follow the
style of other network drivers and so yes, it needs to be written in the
same manner as other drivers. So for example:
+union NI_HV_PT_PORT_STATIC_CFG_t {
+ struct {
+ unsigned int int_cfg : 4; /* bits 3:0 */
+ unsigned int phy_mode : 1; /* bits 4:4 */
+ unsigned int rmii_clksrc : 1; /* bits 5:5 */
+ unsigned int inv_clk_in : 1; /* bits 6:6 */
+ unsigned int inv_clk_out : 1; /* bits 7:7 */
+ unsigned int inv_rxclk_out: 1; /* bits 8:8 */
+ unsigned int tx_use_gefifo: 1; /* bits 9:9 */
+ unsigned int smii_tx_stat : 1; /* bits 10:10 */
+ unsigned int crs_polarity : 1; /* bits 11:11 */
+ unsigned int lpbk_mode: 2; /* bits 13:12 */
+ unsigned int gmii_like_half_duplex_en : 1; /* bits 14:14 */
+ unsigned int sup_tx_to_rx_lpbk_data : 1; /* bits 15:15 */
+ unsigned int rsrvd1 : 8;
+ unsigned int mac_addr6: 8; /* bits 31:24 */
+ } bf;
+ unsigned int wrd;
+};
But in other drivers we just do:
struct pdma_rxd_info2 {
u32 PLEN1 : 14;
u32 LS1 : 1;
u32 UN_USED : 1;
u32 PLEN0 : 14;
u32 LS0 : 1;
u32 DDONE : 1;
};
So yes, all of the places where you have a union we normally do a struct.
>
> > A small item is it looks like this has its own crc function, when we
> > have those available already.
>
> reuse existing CRC functions from u-boot instead of providing our own.
And any other non-IP-specific functionality.
> > There's also things like:
> >> +enum ca_status_t ca_mdio_read(CA_IN unsigned int addr,
> >> +CA_IN unsigned int offset,
> >> +
Re: [PATCH v4 1/2] net: cortina_ni: Add eth support for Cortina Access CAxxxx SoCs
Hi Tom,
> On Jun 3, 2020, at 8:03 AM, Tom Rini wrote:
>
> On Wed, Jun 03, 2020 at 01:05:18AM -0700, Alex Nemirovsky wrote:
>> From: Aaron Tseng
>>
>> Add Cortina Access Ethernet device driver for CA SoCs.
>> This driver supports only the DM_ETH network model.
>>
>> Signed-off-by: Aaron Tseng
>> Signed-off-by: Alex Nemirovsky
>>
>> CC: Joe Hershberger
>> CC: Abbie Chang
>> CC: Tom Rini
>>
>> ---
>>
>> Changes in v4: None
>> Changes in v3:
>> - Changed commit comment to state that only DM model is supported
>> - Removed blank line at end of C file
>>
>> Changes in v2:
>> - Remove legacy mode support
>> - Add support for additional SoC variants
>> - Remove unused variables
>>
>> MAINTAINERS |4 +
>> drivers/net/Kconfig |7 +
>> drivers/net/Makefile |1 +
>> drivers/net/cortina_ni.c | 1909
>> ++
>> drivers/net/cortina_ni.h | 592 ++
>> 5 files changed, 2513 insertions(+)
>> create mode 100644 drivers/net/cortina_ni.c
>> create mode 100644 drivers/net/cortina_ni.h
>
> So, is there a similar driver in upstream Linux?
We don’t have ANY Linux drivers upstream.
Let me see if I can parse the comments below into action items for us.
Let me know if I got it right or missed something.
> This driver doesn't
> quite fit right.
> At an "easy" level, there's still the customer macro
> around debug statements and not using pr_debug(),
find and convert all debug statement to using U-Boot specific pr_debug().
> and contains a whole
> lot of debug code.
remove development debug code which is no longer used.
> There's also code for a saturn platform, is that
> being upstreamed?
Yes, it will be upstreamed to u-boot.The approach is to first focus on getting
all our u-boot drivers upstream for the presidio SoC engineering board.
Most of the drivers are designed to work with other SoC platforms as those
SoC reuse HW logic for our peripherals.
> There's a ton of union usage which is also uncommon.
Is there an action item here?
> A small item is it looks like this has its own crc function, when we
> have those available already.
reuse existing CRC functions from u-boot instead of providing our own.
> There's also things like:
>> +enum ca_status_t ca_mdio_read(CA_IN unsigned int addr,
>> + CA_IN unsigned int offset,
>> + CA_OUT unsigned short*data)
>> +{
>
> Where CA_IN / CA_OUT just don't fit.
What specifically is the action item here?
>
> Which brings me back to why I asked the first question, this feels a lot
> like a driver for an RTOS or some other system was adapted to U-Boot,
> rather than writing a new driver for U-Boot based on internal knowledge
> of the part in question. And I think that applies to a lot of the
> drivers as seen in the NAND review as well. Thanks!
If I understand you correctly, you would like for the drivers to more directly
REUSE
native U-BOOT core functions already provided instead of providing
our own which essentially duplicate the same core functions.
Did I misunderstand anything or do you have something more to add
as action items for us?
>
> --
> Tom
Re: [PATCH v4 1/2] net: cortina_ni: Add eth support for Cortina Access CAxxxx SoCs
On Wed, Jun 03, 2020 at 01:05:18AM -0700, Alex Nemirovsky wrote:
> From: Aaron Tseng
>
> Add Cortina Access Ethernet device driver for CA SoCs.
> This driver supports only the DM_ETH network model.
>
> Signed-off-by: Aaron Tseng
> Signed-off-by: Alex Nemirovsky
>
> CC: Joe Hershberger
> CC: Abbie Chang
> CC: Tom Rini
>
> ---
>
> Changes in v4: None
> Changes in v3:
> - Changed commit comment to state that only DM model is supported
> - Removed blank line at end of C file
>
> Changes in v2:
> - Remove legacy mode support
> - Add support for additional SoC variants
> - Remove unused variables
>
> MAINTAINERS |4 +
> drivers/net/Kconfig |7 +
> drivers/net/Makefile |1 +
> drivers/net/cortina_ni.c | 1909
> ++
> drivers/net/cortina_ni.h | 592 ++
> 5 files changed, 2513 insertions(+)
> create mode 100644 drivers/net/cortina_ni.c
> create mode 100644 drivers/net/cortina_ni.h
So, is there a similar driver in upstream Linux? This driver doesn't
quite fit right. At an "easy" level, there's still the customer macro
around debug statements and not using pr_debug(), and contains a whole
lot of debug code. There's also code for a saturn platform, is that
being upstreamed? There's a ton of union usage which is also uncommon.
A small item is it looks like this has its own crc function, when we
have those available already. There's also things like:
> +enum ca_status_t ca_mdio_read(CA_IN unsigned int addr,
> + CA_IN unsigned int offset,
> + CA_OUT unsigned short*data)
> +{
Where CA_IN / CA_OUT just don't fit.
Which brings me back to why I asked the first question, this feels a lot
like a driver for an RTOS or some other system was adapted to U-Boot,
rather than writing a new driver for U-Boot based on internal knowledge
of the part in question. And I think that applies to a lot of the
drivers as seen in the NAND review as well. Thanks!
--
Tom
signature.asc
Description: PGP signature
[PATCH v4 1/2] net: cortina_ni: Add eth support for Cortina Access CAxxxx SoCs
From: Aaron Tseng
Add Cortina Access Ethernet device driver for CA SoCs.
This driver supports only the DM_ETH network model.
Signed-off-by: Aaron Tseng
Signed-off-by: Alex Nemirovsky
CC: Joe Hershberger
CC: Abbie Chang
CC: Tom Rini
---
Changes in v4: None
Changes in v3:
- Changed commit comment to state that only DM model is supported
- Removed blank line at end of C file
Changes in v2:
- Remove legacy mode support
- Add support for additional SoC variants
- Remove unused variables
MAINTAINERS |4 +
drivers/net/Kconfig |7 +
drivers/net/Makefile |1 +
drivers/net/cortina_ni.c | 1909 ++
drivers/net/cortina_ni.h | 592 ++
5 files changed, 2513 insertions(+)
create mode 100644 drivers/net/cortina_ni.c
create mode 100644 drivers/net/cortina_ni.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 8add9d4..1b166d2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -181,6 +181,8 @@ F: drivers/gpio/cortina_gpio.c
F: drivers/watchdog/cortina_wdt.c
F: drivers/serial/serial_cortina.c
F: drivers/mmc/ca_dw_mmc.c
+F: drivers/net/cortina_ni.c
+F: drivers/net/cortina_ni.h
ARM/CZ.NIC TURRIS MOX SUPPORT
M: Marek Behun
@@ -732,6 +734,8 @@ F: drivers/gpio/cortina_gpio.c
F: drivers/watchdog/cortina_wdt.c
F: drivers/serial/serial_cortina.c
F: drivers/mmc/ca_dw_mmc.c
+F: drivers/net/cortina_ni.c
+F: drivers/net/cortina_ni.h
MIPS MSCC
M: Gregory CLEMENT
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index f7855c9..45e0480 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -149,6 +149,13 @@ config BCMGENET
help
This driver supports the BCMGENET Ethernet MAC.
+config CORTINA_NI_ENET
+ bool "Cortina-Access Ethernet driver"
+ depends on DM_ETH && CORTINA_PLATFORM
+ help
+ The driver supports the Cortina-Access Ethernet MAC for
+ all supported CA SoCs
+
config DWC_ETH_QOS
bool "Synopsys DWC Ethernet QOS device support"
depends on DM_ETH
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index 383ed1c..1d6ec4f 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_DRIVER_AX88180) += ax88180.o
obj-$(CONFIG_BCM_SF2_ETH) += bcm-sf2-eth.o
obj-$(CONFIG_BCM_SF2_ETH_GMAC) += bcm-sf2-eth-gmac.o
obj-$(CONFIG_CALXEDA_XGMAC) += calxedaxgmac.o
+obj-$(CONFIG_CORTINA_NI_ENET) += cortina_ni.o
obj-$(CONFIG_CS8900) += cs8900.o
obj-$(CONFIG_TULIP) += dc2114x.o
obj-$(CONFIG_ETH_DESIGNWARE) += designware.o
diff --git a/drivers/net/cortina_ni.c b/drivers/net/cortina_ni.c
new file mode 100644
index 000..d5bbf3e
--- /dev/null
+++ b/drivers/net/cortina_ni.c
@@ -0,0 +1,1909 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/*
+ * Copyright (C) 2020 Cortina Access Inc.
+ * Author: Aaron Tseng
+ *
+ * Ethernet MAC Driver for all supported CA SoCs
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "cortina_ni.h"
+
+static u32 reg_value;
+
+/* port 0-3 are individual port connect to PHY directly */
+/* port 4-7 are LAN ports connected to QSGMII PHY */
+int active_port = NI_PORT_5; /* Physical port 5 */
+u32 ge_port_phy_addr; /* PHY address connected to active port */
+int auto_scan_active_port;
+
+#define HEADER_A_SIZE 8
+
+/*define CORTINA_NI_DBG if individual rx,tx,init needs to be called */
+#if CORTINA_NI_DBG
+static struct udevice *dbg_dev;
+#endif
+static struct udevice *curr_dev;
+
+#if defined(CONFIG_TARGET_SATURN_ASIC)
+#define CA_REG_READ(off)readl((u64)KSEG1_ATU_XLAT(off))
+#define CA_REG_WRITE(data, off) writel(data, (u64)KSEG1_ATU_XLAT(off))
+#else
+#define CA_REG_READ(off)readl((u64)off)
+#define CA_REG_WRITE(data, off) writel(data, (u64)off)
+#endif
+
+int cortina_ni_recv(struct udevice *netdev);
+static int ca_ni_ofdata_to_platdata(struct udevice *dev);
+
+static u32 *RDWRPTR_ADVANCE_ONE(u32 *x, unsigned long base, unsigned long max)
+{
+ if (x + 1 >= (u32 *)max)
+ return (u32 *)base;
+ else
+ return (x + 1);
+}
+
+static void ni_setup_mac_addr(void)
+{
+ unsigned char mac[6];
+
+ union NI_HV_GLB_MAC_ADDR_CFG0_t mac_addr_cfg0;
+ union NI_HV_GLB_MAC_ADDR_CFG1_t mac_addr_cfg1;
+ union NI_HV_PT_PORT_STATIC_CFG_tport_static_cfg;
+ union NI_HV_XRAM_CPUXRAM_CFG_t cpuxram_cfg;
+ struct cortina_ni_priv *priv = dev_get_priv(curr_dev);
+
+ /* parsing ethaddr and set to NI registers. */
+ if (eth_env_get_enetaddr("ethaddr", mac)) {
+ /* The complete MAC address consists of
+* {MAC_ADDR0_mac_addr0[0-3], MAC_ADDR1_mac_addr1[4],
+* PT_PORT_STATIC_CFG_mac_addr6[5]}.
+*/
+ mac_addr_cfg0.bf.mac_addr0 = (mac[0] << 24) +
+(mac[1] << 16) +
+
