Message ID | 20210802102654.5996-2-biju.das.jz@bp.renesas.com (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | Add Gigabit Ethernet driver support | expand |
Context | Check | Description |
---|---|---|
netdev/cover_letter | success | Link |
netdev/fixes_present | success | Link |
netdev/patch_count | success | Link |
netdev/tree_selection | success | Clearly marked for net-next |
netdev/subject_prefix | success | Link |
netdev/cc_maintainers | warning | 3 maintainers not CCed: yangyingliang@huawei.com rikard.falkeborn@gmail.com f.fainelli@gmail.com |
netdev/source_inline | success | Was 0 now: 0 |
netdev/verify_signedoff | success | Link |
netdev/module_param | success | Was 0 now: 0 |
netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
netdev/verify_fixes | success | Link |
netdev/checkpatch | warning | WARNING: line length of 81 exceeds 80 columns WARNING: line length of 83 exceeds 80 columns |
netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
netdev/header_inline | success | Link |
On Mon, Aug 02, 2021 at 11:26:47AM +0100, Biju Das wrote: > The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC are > similar to the R-Car Ethernet AVB IP. With a few changes in the driver we > can support both IPs. > > Currently a runtime decision based on the chip type is used to distinguish > the HW differences between the SoC families. > > The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car Gen2 and > RZ/G2L it is 2. For cases like this it is better to select the number of > TX descriptors by using a structure with a value, rather than a runtime > decision based on the chip type. > > This patch adds the num_tx_desc variable to struct ravb_hw_info and also > replaces the driver data chip type with struct ravb_hw_info by moving chip > type to it. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Hi Biju This is better. A lot clearer what is going on. I personally would of done the num_tx_desc change as a separate patch, but this is O.K. Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
On 8/2/21 1:26 PM, Biju Das wrote: > The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC are > similar to the R-Car Ethernet AVB IP. With a few changes in the driver we > can support both IPs. > > Currently a runtime decision based on the chip type is used to distinguish > the HW differences between the SoC families. > > The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car Gen2 and > RZ/G2L it is 2. For cases like this it is better to select the number of > TX descriptors by using a structure with a value, rather than a runtime > decision based on the chip type. > > This patch adds the num_tx_desc variable to struct ravb_hw_info and also > replaces the driver data chip type with struct ravb_hw_info by moving chip > type to it. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > --- > v2: > * Incorporated Andrew and Sergei's review comments for making it smaller patch > and provided detailed description. > --- > drivers/net/ethernet/renesas/ravb.h | 7 +++++ > drivers/net/ethernet/renesas/ravb_main.c | 38 +++++++++++++++--------- > 2 files changed, 31 insertions(+), 14 deletions(-) > > diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h > index 80e62ca2e3d3..cfb972c05b34 100644 > --- a/drivers/net/ethernet/renesas/ravb.h > +++ b/drivers/net/ethernet/renesas/ravb.h > @@ -988,6 +988,11 @@ enum ravb_chip_id { > RCAR_GEN3, > }; > > +struct ravb_hw_info { > + enum ravb_chip_id chip_id; > + int num_tx_desc; I think this is rather the driver's choice, than the h/w feature... Perhaps a rename would help with that? :-) [...] MBR, Sergei
Hi Sergei, Thanks for the feedback. > Subject: Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to > driver data > > On 8/2/21 1:26 PM, Biju Das wrote: > > > The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC > > are similar to the R-Car Ethernet AVB IP. With a few changes in the > > driver we can support both IPs. > > > > Currently a runtime decision based on the chip type is used to > > distinguish the HW differences between the SoC families. > > > > The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car Gen2 > > and RZ/G2L it is 2. For cases like this it is better to select the > > number of TX descriptors by using a structure with a value, rather > > than a runtime decision based on the chip type. > > > > This patch adds the num_tx_desc variable to struct ravb_hw_info and > > also replaces the driver data chip type with struct ravb_hw_info by > > moving chip type to it. > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > --- > > v2: > > * Incorporated Andrew and Sergei's review comments for making it > smaller patch > > and provided detailed description. > > --- > > drivers/net/ethernet/renesas/ravb.h | 7 +++++ > > drivers/net/ethernet/renesas/ravb_main.c | 38 > > +++++++++++++++--------- > > 2 files changed, 31 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/net/ethernet/renesas/ravb.h > > b/drivers/net/ethernet/renesas/ravb.h > > index 80e62ca2e3d3..cfb972c05b34 100644 > > --- a/drivers/net/ethernet/renesas/ravb.h > > +++ b/drivers/net/ethernet/renesas/ravb.h > > @@ -988,6 +988,11 @@ enum ravb_chip_id { > > RCAR_GEN3, > > }; > > > > +struct ravb_hw_info { > > + enum ravb_chip_id chip_id; > > + int num_tx_desc; > > I think this is rather the driver's choice, than the h/w feature... > Perhaps a rename would help with that? :-) It is consistent with current naming convention used by the driver. NUM_TX_DESC macro is replaced by num_tx_desc and the below run time decision based on chip type for H/W configuration for Gen2/Gen3 is replaced by info->num_tx_desc. priv->num_tx_desc = chip_id == RCAR_GEN2 ? NUM_TX_DESC_GEN2 : NUM_TX_DESC_GEN3; Please let me know, if I am missing anything, Previously there is a suggestion to change the generic struct ravb_driver_data(which holds driver differences and HW features) with struct ravb_hw_info. Regards, Biju > > [...] > > MBR, Sergei
Hi Sergei, > Subject: RE: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to > driver data > > Hi Sergei, > > Thanks for the feedback. > > > Subject: Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to > > driver data > > > > On 8/2/21 1:26 PM, Biju Das wrote: > > > > > The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC > > > are similar to the R-Car Ethernet AVB IP. With a few changes in the > > > driver we can support both IPs. > > > > > > Currently a runtime decision based on the chip type is used to > > > distinguish the HW differences between the SoC families. > > > > > > The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car > > > Gen2 and RZ/G2L it is 2. For cases like this it is better to select > > > the number of TX descriptors by using a structure with a value, > > > rather than a runtime decision based on the chip type. > > > > > > This patch adds the num_tx_desc variable to struct ravb_hw_info and > > > also replaces the driver data chip type with struct ravb_hw_info by > > > moving chip type to it. > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > --- > > > v2: > > > * Incorporated Andrew and Sergei's review comments for making it > > smaller patch > > > and provided detailed description. > > > --- > > > drivers/net/ethernet/renesas/ravb.h | 7 +++++ > > > drivers/net/ethernet/renesas/ravb_main.c | 38 > > > +++++++++++++++--------- > > > 2 files changed, 31 insertions(+), 14 deletions(-) > > > > > > diff --git a/drivers/net/ethernet/renesas/ravb.h > > > b/drivers/net/ethernet/renesas/ravb.h > > > index 80e62ca2e3d3..cfb972c05b34 100644 > > > --- a/drivers/net/ethernet/renesas/ravb.h > > > +++ b/drivers/net/ethernet/renesas/ravb.h > > > @@ -988,6 +988,11 @@ enum ravb_chip_id { > > > RCAR_GEN3, > > > }; > > > > > > +struct ravb_hw_info { > > > + enum ravb_chip_id chip_id; > > > + int num_tx_desc; > > > > I think this is rather the driver's choice, than the h/w feature... > > Perhaps a rename would help with that? :-) > > It is consistent with current naming convention used by the driver. > NUM_TX_DESC macro is replaced by num_tx_desc. So the name should be ok. Indeed we are agreed to add function pointers to struct ravb_hw_info to avoid another level of indirection. If the concern is related to duplication of data(ie,priv->num_tx_desc vs info->num_tx_desc) I have a plan to remove priv->num_tx_desc with info->num_tx_desc later. Regards, Biju
On 8/3/21 8:57 AM, Biju Das wrote: >> Subject: Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to >> driver data >> >> On 8/2/21 1:26 PM, Biju Das wrote: >> >>> The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC >>> are similar to the R-Car Ethernet AVB IP. With a few changes in the >>> driver we can support both IPs. >>> >>> Currently a runtime decision based on the chip type is used to >>> distinguish the HW differences between the SoC families. >>> >>> The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car Gen2 >>> and RZ/G2L it is 2. For cases like this it is better to select the >>> number of TX descriptors by using a structure with a value, rather >>> than a runtime decision based on the chip type. >>> >>> This patch adds the num_tx_desc variable to struct ravb_hw_info and >>> also replaces the driver data chip type with struct ravb_hw_info by >>> moving chip type to it. >>> >>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> >>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> >>> --- >>> v2: >>> * Incorporated Andrew and Sergei's review comments for making it >> smaller patch >>> and provided detailed description. >>> --- >>> drivers/net/ethernet/renesas/ravb.h | 7 +++++ >>> drivers/net/ethernet/renesas/ravb_main.c | 38 >>> +++++++++++++++--------- >>> 2 files changed, 31 insertions(+), 14 deletions(-) >>> >>> diff --git a/drivers/net/ethernet/renesas/ravb.h >>> b/drivers/net/ethernet/renesas/ravb.h >>> index 80e62ca2e3d3..cfb972c05b34 100644 >>> --- a/drivers/net/ethernet/renesas/ravb.h >>> +++ b/drivers/net/ethernet/renesas/ravb.h >>> @@ -988,6 +988,11 @@ enum ravb_chip_id { >>> RCAR_GEN3, >>> }; >>> >>> +struct ravb_hw_info { >>> + enum ravb_chip_id chip_id; >>> + int num_tx_desc; How about leaving that field in the *struct* ravb_private? And adding the following instead: unsigned unaligned_tx: 1; >> I think this is rather the driver's choice, than the h/w feature... >> Perhaps a rename would help with that? :-) > > It is consistent with current naming convention used by the driver. NUM_TX_DESC macro is replaced by num_tx_desc and the below run time decision based on chip type for H/W configuration for Gen2/Gen3 is replaced by info->num_tx_desc. > > priv->num_tx_desc = chip_id == RCAR_GEN2 ? NUM_TX_DESC_GEN2 : NUM_TX_DESC_GEN3; .. and then: priv->num_tx_desc = info->unaligned_tx ? NUM_TX_DESC_GEN2 : NUM_TX_DESC_GEN3; > Please let me know, if I am missing anything, > > Previously there is a suggestion to change the generic struct ravb_driver_data (which holds driver differences and HW features) with struct ravb_hw_info. Well, my plan was to place all the hardware features supported into the *struct* ravb_hw_info and leave all the driver's software data in the *struct* ravb_private. > Regards, > Biju [...] MBR, Sergei
On 8/4/21 10:27 PM, Sergei Shtylyov wrote: >>> Subject: Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to >>> driver data >>> >>> On 8/2/21 1:26 PM, Biju Das wrote: >>> >>>> The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC >>>> are similar to the R-Car Ethernet AVB IP. With a few changes in the >>>> driver we can support both IPs. >>>> >>>> Currently a runtime decision based on the chip type is used to >>>> distinguish the HW differences between the SoC families. >>>> >>>> The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car Gen2 >>>> and RZ/G2L it is 2. For cases like this it is better to select the >>>> number of TX descriptors by using a structure with a value, rather >>>> than a runtime decision based on the chip type. >>>> >>>> This patch adds the num_tx_desc variable to struct ravb_hw_info and >>>> also replaces the driver data chip type with struct ravb_hw_info by >>>> moving chip type to it. >>>> >>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> >>>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> >>>> --- >>>> v2: >>>> * Incorporated Andrew and Sergei's review comments for making it >>> smaller patch >>>> and provided detailed description. >>>> --- >>>> drivers/net/ethernet/renesas/ravb.h | 7 +++++ >>>> drivers/net/ethernet/renesas/ravb_main.c | 38 >>>> +++++++++++++++--------- >>>> 2 files changed, 31 insertions(+), 14 deletions(-) >>>> >>>> diff --git a/drivers/net/ethernet/renesas/ravb.h >>>> b/drivers/net/ethernet/renesas/ravb.h >>>> index 80e62ca2e3d3..cfb972c05b34 100644 >>>> --- a/drivers/net/ethernet/renesas/ravb.h >>>> +++ b/drivers/net/ethernet/renesas/ravb.h >>>> @@ -988,6 +988,11 @@ enum ravb_chip_id { >>>> RCAR_GEN3, >>>> }; >>>> >>>> +struct ravb_hw_info { >>>> + enum ravb_chip_id chip_id; >>>> + int num_tx_desc; > > How about leaving that field in the *struct* ravb_private? And adding the following instead: > > unsigned unaligned_tx: 1; Or aligned_tx, so that gen2 has it set, and gen3 has it cleared. > >>> I think this is rather the driver's choice, than the h/w feature... >>> Perhaps a rename would help with that? :-) >> >> It is consistent with current naming convention used by the driver. NUM_TX_DESC macro is replaced by num_tx_desc and the below run time decision based on chip type for H/W configuration for Gen2/Gen3 is replaced by info->num_tx_desc. >> >> priv->num_tx_desc = chip_id == RCAR_GEN2 ? NUM_TX_DESC_GEN2 : NUM_TX_DESC_GEN3; > > .. and then: > > priv->num_tx_desc = info->unaligned_tx ? NUM_TX_DESC_GEN2 : NUM_TX_DESC_GEN3; Sorry, mixed the values, should have been: priv->num_tx_desc = info->unaligned_tx ? NUM_TX_DESC_GEN3 : NUM_TX_DESC_GEN2; >> Please let me know, if I am missing anything, >> >> Previously there is a suggestion to change the generic struct ravb_driver_data (which holds driver differences and HW features) with struct ravb_hw_info. > > Well, my plan was to place all the hardware features supported into the *struct* ravb_hw_info and leave all > the driver's software data in the *struct* ravb_private. ... just like *struct* sh_eth_cpu_data and sh_eth_private in the sh_eth driver. >> Regards, >> Biju MBR, Sergei
Hi Biju, On Mon, Aug 2, 2021 at 12:27 PM Biju Das <biju.das.jz@bp.renesas.com> wrote: > The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC are > similar to the R-Car Ethernet AVB IP. With a few changes in the driver we > can support both IPs. > > Currently a runtime decision based on the chip type is used to distinguish > the HW differences between the SoC families. > > The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car Gen2 and > RZ/G2L it is 2. For cases like this it is better to select the number of > TX descriptors by using a structure with a value, rather than a runtime > decision based on the chip type. > > This patch adds the num_tx_desc variable to struct ravb_hw_info and also > replaces the driver data chip type with struct ravb_hw_info by moving chip > type to it. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Thanks for your patch! > --- a/drivers/net/ethernet/renesas/ravb.h > +++ b/drivers/net/ethernet/renesas/ravb.h > @@ -988,6 +988,11 @@ enum ravb_chip_id { > RCAR_GEN3, > }; > > +struct ravb_hw_info { > + enum ravb_chip_id chip_id; > + int num_tx_desc; Why not "unsigned int"? ... This comment applies to a few more subsequent patches. > +}; > + > struct ravb_private { > struct net_device *ndev; > struct platform_device *pdev; > @@ -1040,6 +1045,8 @@ struct ravb_private { > unsigned txcidm:1; /* TX Clock Internal Delay Mode */ > unsigned rgmii_override:1; /* Deprecated rgmii-*id behavior */ > int num_tx_desc; /* TX descriptors per packet */ ... oh, here's the original culprit. > + > + const struct ravb_hw_info *info; > }; > Gr{oetje,eeting}s, Geert
Hi Geert, Thanks for the feedback. > -----Original Message----- > Subject: Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to > driver data > > Hi Biju, > > On Mon, Aug 2, 2021 at 12:27 PM Biju Das <biju.das.jz@bp.renesas.com> > wrote: > > The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC > > are similar to the R-Car Ethernet AVB IP. With a few changes in the > > driver we can support both IPs. > > > > Currently a runtime decision based on the chip type is used to > > distinguish the HW differences between the SoC families. > > > > The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car Gen2 > > and RZ/G2L it is 2. For cases like this it is better to select the > > number of TX descriptors by using a structure with a value, rather > > than a runtime decision based on the chip type. > > > > This patch adds the num_tx_desc variable to struct ravb_hw_info and > > also replaces the driver data chip type with struct ravb_hw_info by > > moving chip type to it. > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Thanks for your patch! > > > --- a/drivers/net/ethernet/renesas/ravb.h > > +++ b/drivers/net/ethernet/renesas/ravb.h > > @@ -988,6 +988,11 @@ enum ravb_chip_id { > > RCAR_GEN3, > > }; > > > > +struct ravb_hw_info { > > + enum ravb_chip_id chip_id; > > + int num_tx_desc; > > Why not "unsigned int"? ... > This comment applies to a few more subsequent patches. To avoid signed and unsigned comparison warnings. > > > +}; > > + > > struct ravb_private { > > struct net_device *ndev; > > struct platform_device *pdev; > > @@ -1040,6 +1045,8 @@ struct ravb_private { > > unsigned txcidm:1; /* TX Clock Internal Delay Mode > */ > > unsigned rgmii_override:1; /* Deprecated rgmii-*id behavior > */ > > int num_tx_desc; /* TX descriptors per packet */ > > ... oh, here's the original culprit. Exactly, this the reason. Do you want me to change this into unsigned int? Please let me know. Regards, Biju > > > + > > + const struct ravb_hw_info *info; > > }; > > > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux- > m68k.org > > In personal conversations with technical people, I call myself a hacker. > But when I'm talking to journalists I just say "programmer" or something > like that. > -- Linus Torvalds
Hi Biju, On Thu, Aug 12, 2021 at 9:26 AM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > -----Original Message----- > > On Mon, Aug 2, 2021 at 12:27 PM Biju Das <biju.das.jz@bp.renesas.com> > > wrote: > > > The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC > > > are similar to the R-Car Ethernet AVB IP. With a few changes in the > > > driver we can support both IPs. > > > > > > Currently a runtime decision based on the chip type is used to > > > distinguish the HW differences between the SoC families. > > > > > > The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car Gen2 > > > and RZ/G2L it is 2. For cases like this it is better to select the > > > number of TX descriptors by using a structure with a value, rather > > > than a runtime decision based on the chip type. > > > > > > This patch adds the num_tx_desc variable to struct ravb_hw_info and > > > also replaces the driver data chip type with struct ravb_hw_info by > > > moving chip type to it. > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > Thanks for your patch! > > > > > --- a/drivers/net/ethernet/renesas/ravb.h > > > +++ b/drivers/net/ethernet/renesas/ravb.h > > > @@ -988,6 +988,11 @@ enum ravb_chip_id { > > > RCAR_GEN3, > > > }; > > > > > > +struct ravb_hw_info { > > > + enum ravb_chip_id chip_id; > > > + int num_tx_desc; > > > > Why not "unsigned int"? ... > > This comment applies to a few more subsequent patches. > > To avoid signed and unsigned comparison warnings. > > > > > > +}; > > > + > > > struct ravb_private { > > > struct net_device *ndev; > > > struct platform_device *pdev; > > > @@ -1040,6 +1045,8 @@ struct ravb_private { > > > unsigned txcidm:1; /* TX Clock Internal Delay Mode > > */ > > > unsigned rgmii_override:1; /* Deprecated rgmii-*id behavior > > */ > > > int num_tx_desc; /* TX descriptors per packet */ > > > > ... oh, here's the original culprit. > > Exactly, this the reason. > > Do you want me to change this into unsigned int? Please let me know. Up to you (or the maintainer? ;-) For new fields (in the other patches), I would use unsigned for all unsigned values. Signed values have more pitfalls related to undefined behavior. Gr{oetje,eeting}s, Geert
Hi Geert, Thanks for the feedback. > Subject: Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to > driver data > > Hi Biju, > > On Thu, Aug 12, 2021 at 9:26 AM Biju Das <biju.das.jz@bp.renesas.com> > wrote: > > > -----Original Message----- > > > On Mon, Aug 2, 2021 at 12:27 PM Biju Das > > > <biju.das.jz@bp.renesas.com> > > > wrote: > > > > The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L > > > > SoC are similar to the R-Car Ethernet AVB IP. With a few changes > > > > in the driver we can support both IPs. > > > > > > > > Currently a runtime decision based on the chip type is used to > > > > distinguish the HW differences between the SoC families. > > > > > > > > The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car > > > > Gen2 and RZ/G2L it is 2. For cases like this it is better to > > > > select the number of TX descriptors by using a structure with a > > > > value, rather than a runtime decision based on the chip type. > > > > > > > > This patch adds the num_tx_desc variable to struct ravb_hw_info > > > > and also replaces the driver data chip type with struct > > > > ravb_hw_info by moving chip type to it. > > > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > Reviewed-by: Lad Prabhakar > > > > <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > > > Thanks for your patch! > > > > > > > --- a/drivers/net/ethernet/renesas/ravb.h > > > > +++ b/drivers/net/ethernet/renesas/ravb.h > > > > @@ -988,6 +988,11 @@ enum ravb_chip_id { > > > > RCAR_GEN3, > > > > }; > > > > > > > > +struct ravb_hw_info { > > > > + enum ravb_chip_id chip_id; > > > > + int num_tx_desc; > > > > > > Why not "unsigned int"? ... > > > This comment applies to a few more subsequent patches. > > > > To avoid signed and unsigned comparison warnings. > > > > > > > > > +}; > > > > + > > > > struct ravb_private { > > > > struct net_device *ndev; > > > > struct platform_device *pdev; @@ -1040,6 +1045,8 @@ struct > > > > ravb_private { > > > > unsigned txcidm:1; /* TX Clock Internal Delay > Mode > > > */ > > > > unsigned rgmii_override:1; /* Deprecated rgmii-*id > behavior > > > */ > > > > int num_tx_desc; /* TX descriptors per packet > */ > > > > > > ... oh, here's the original culprit. > > > > Exactly, this the reason. > > > > Do you want me to change this into unsigned int? Please let me know. > > Up to you (or the maintainer? ;-) > > For new fields (in the other patches), I would use unsigned for all > unsigned values. Signed values have more pitfalls related to undefined > behavior. Sergei, What is your thoughts here? Please let me know. Cheers, Biju
Hi all, > Subject: RE: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to > driver data > > Hi Geert, > > Thanks for the feedback. > > > Subject: Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to > > driver data > > > > Hi Biju, > > > > On Thu, Aug 12, 2021 at 9:26 AM Biju Das <biju.das.jz@bp.renesas.com> > > wrote: > > > > -----Original Message----- > > > > On Mon, Aug 2, 2021 at 12:27 PM Biju Das > > > > <biju.das.jz@bp.renesas.com> > > > > wrote: > > > > > The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L > > > > > SoC are similar to the R-Car Ethernet AVB IP. With a few changes > > > > > in the driver we can support both IPs. > > > > > > > > > > Currently a runtime decision based on the chip type is used to > > > > > distinguish the HW differences between the SoC families. > > > > > > > > > > The number of TX descriptors for R-Car Gen3 is 1 whereas on > > > > > R-Car > > > > > Gen2 and RZ/G2L it is 2. For cases like this it is better to > > > > > select the number of TX descriptors by using a structure with a > > > > > value, rather than a runtime decision based on the chip type. > > > > > > > > > > This patch adds the num_tx_desc variable to struct ravb_hw_info > > > > > and also replaces the driver data chip type with struct > > > > > ravb_hw_info by moving chip type to it. > > > > > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > > Reviewed-by: Lad Prabhakar > > > > > <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > > > > > Thanks for your patch! > > > > > > > > > --- a/drivers/net/ethernet/renesas/ravb.h > > > > > +++ b/drivers/net/ethernet/renesas/ravb.h > > > > > @@ -988,6 +988,11 @@ enum ravb_chip_id { > > > > > RCAR_GEN3, > > > > > }; > > > > > > > > > > +struct ravb_hw_info { > > > > > + enum ravb_chip_id chip_id; > > > > > + int num_tx_desc; > > > > > > > > Why not "unsigned int"? ... > > > > This comment applies to a few more subsequent patches. > > > > > > To avoid signed and unsigned comparison warnings. > > > > > > > > > > > > +}; > > > > > + > > > > > struct ravb_private { > > > > > struct net_device *ndev; > > > > > struct platform_device *pdev; @@ -1040,6 +1045,8 @@ > > > > > struct ravb_private { > > > > > unsigned txcidm:1; /* TX Clock Internal Delay > > Mode > > > > */ > > > > > unsigned rgmii_override:1; /* Deprecated rgmii-*id > > behavior > > > > */ > > > > > int num_tx_desc; /* TX descriptors per > packet > > */ > > > > > > > > ... oh, here's the original culprit. > > > > > > Exactly, this the reason. > > > > > > Do you want me to change this into unsigned int? Please let me know. > > > > Up to you (or the maintainer? ;-) > > > > For new fields (in the other patches), I would use unsigned for all > > unsigned values. Signed values have more pitfalls related to > > undefined behavior. > > Sergei, What is your thoughts here? Please let me know. Here is my plan. I will split this patch into two as Andrew suggested and Then on the second patch will add as info->unaligned_tx as Sergei suggested. Now the only open point is related to the data type of "int num_tx_desc" and to align with sh_eth driver I will keep int. Regards, Biju
On 8/17/21 2:24 PM, Biju Das wrote: [...] >>>>> -----Original Message----- >>>>> On Mon, Aug 2, 2021 at 12:27 PM Biju Das >>>>> <biju.das.jz@bp.renesas.com> >>>>> wrote: >>>>>> The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L >>>>>> SoC are similar to the R-Car Ethernet AVB IP. With a few changes >>>>>> in the driver we can support both IPs. >>>>>> >>>>>> Currently a runtime decision based on the chip type is used to >>>>>> distinguish the HW differences between the SoC families. >>>>>> >>>>>> The number of TX descriptors for R-Car Gen3 is 1 whereas on >>>>>> R-Car >>>>>> Gen2 and RZ/G2L it is 2. For cases like this it is better to >>>>>> select the number of TX descriptors by using a structure with a >>>>>> value, rather than a runtime decision based on the chip type. >>>>>> >>>>>> This patch adds the num_tx_desc variable to struct ravb_hw_info >>>>>> and also replaces the driver data chip type with struct >>>>>> ravb_hw_info by moving chip type to it. >>>>>> >>>>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> >>>>>> Reviewed-by: Lad Prabhakar >>>>>> <prabhakar.mahadev-lad.rj@bp.renesas.com> >>>>> >>>>> Thanks for your patch! >>>>> >>>>>> --- a/drivers/net/ethernet/renesas/ravb.h >>>>>> +++ b/drivers/net/ethernet/renesas/ravb.h >>>>>> @@ -988,6 +988,11 @@ enum ravb_chip_id { >>>>>> RCAR_GEN3, >>>>>> }; >>>>>> >>>>>> +struct ravb_hw_info { >>>>>> + enum ravb_chip_id chip_id; >>>>>> + int num_tx_desc; >>>>> >>>>> Why not "unsigned int"? ... >>>>> This comment applies to a few more subsequent patches. >>>> >>>> To avoid signed and unsigned comparison warnings. >>>> >>>>> >>>>>> +}; >>>>>> + >>>>>> struct ravb_private { >>>>>> struct net_device *ndev; >>>>>> struct platform_device *pdev; @@ -1040,6 +1045,8 @@ >>>>>> struct ravb_private { >>>>>> unsigned txcidm:1; /* TX Clock Internal Delay >>> Mode >>>>> */ >>>>>> unsigned rgmii_override:1; /* Deprecated rgmii-*id >>> behavior >>>>> */ >>>>>> int num_tx_desc; /* TX descriptors per >> packet >>> */ >>>>> >>>>> ... oh, here's the original culprit. >>>> >>>> Exactly, this the reason. >>>> >>>> Do you want me to change this into unsigned int? Please let me know. >>> >>> Up to you (or the maintainer? ;-) >>> >>> For new fields (in the other patches), I would use unsigned for all >>> unsigned values. Signed values have more pitfalls related to >>> undefined behavior. >> >> Sergei, What is your thoughts here? Please let me know. > > Here is my plan. > > I will split this patch into two as Andrew suggested and If you mran changing the ravb_private::num_tx_desc to *unsigned*, it'll be a good cleanup. What's would be the 2nd part tho? > Then on the second patch will add as info->unaligned_tx as Sergei suggested. OK. > Now the only open point is related to the data type of "int num_tx_desc" > and to align with sh_eth driver I will keep int. The sh_eth driver simply doesn't have this -- it always use 1 descriptor. > Regards, > Biju MBR, Sergey
Hi Sergei, Thanks for the feedback. > Subject: Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to > driver data > > On 8/17/21 2:24 PM, Biju Das wrote: > > [...] > >>>>> -----Original Message----- > >>>>> On Mon, Aug 2, 2021 at 12:27 PM Biju Das > >>>>> <biju.das.jz@bp.renesas.com> > >>>>> wrote: > >>>>>> The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L > >>>>>> SoC are similar to the R-Car Ethernet AVB IP. With a few changes > >>>>>> in the driver we can support both IPs. > >>>>>> > >>>>>> Currently a runtime decision based on the chip type is used to > >>>>>> distinguish the HW differences between the SoC families. > >>>>>> > >>>>>> The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car > >>>>>> Gen2 and RZ/G2L it is 2. For cases like this it is better to > >>>>>> select the number of TX descriptors by using a structure with a > >>>>>> value, rather than a runtime decision based on the chip type. > >>>>>> > >>>>>> This patch adds the num_tx_desc variable to struct ravb_hw_info > >>>>>> and also replaces the driver data chip type with struct > >>>>>> ravb_hw_info by moving chip type to it. > >>>>>> > >>>>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > >>>>>> Reviewed-by: Lad Prabhakar > >>>>>> <prabhakar.mahadev-lad.rj@bp.renesas.com> > >>>>> > >>>>> Thanks for your patch! > >>>>> > >>>>>> --- a/drivers/net/ethernet/renesas/ravb.h > >>>>>> +++ b/drivers/net/ethernet/renesas/ravb.h > >>>>>> @@ -988,6 +988,11 @@ enum ravb_chip_id { > >>>>>> RCAR_GEN3, > >>>>>> }; > >>>>>> > >>>>>> +struct ravb_hw_info { > >>>>>> + enum ravb_chip_id chip_id; > >>>>>> + int num_tx_desc; > >>>>> > >>>>> Why not "unsigned int"? ... > >>>>> This comment applies to a few more subsequent patches. > >>>> > >>>> To avoid signed and unsigned comparison warnings. > >>>> > >>>>> > >>>>>> +}; > >>>>>> + > >>>>>> struct ravb_private { > >>>>>> struct net_device *ndev; > >>>>>> struct platform_device *pdev; @@ -1040,6 +1045,8 @@ > >>>>>> struct ravb_private { > >>>>>> unsigned txcidm:1; /* TX Clock Internal Delay > >>> Mode > >>>>> */ > >>>>>> unsigned rgmii_override:1; /* Deprecated rgmii-*id > >>> behavior > >>>>> */ > >>>>>> int num_tx_desc; /* TX descriptors per > >> packet > >>> */ > >>>>> > >>>>> ... oh, here's the original culprit. > >>>> > >>>> Exactly, this the reason. > >>>> > >>>> Do you want me to change this into unsigned int? Please let me know. > >>> > >>> Up to you (or the maintainer? ;-) > >>> > >>> For new fields (in the other patches), I would use unsigned for all > >>> unsigned values. Signed values have more pitfalls related to > >>> undefined behavior. > >> > >> Sergei, What is your thoughts here? Please let me know. > > > > Here is my plan. > > > > I will split this patch into two as Andrew suggested and > > If you mran changing the ravb_private::num_tx_desc to *unsigned*, it'll > be a good cleanup. What's would be the 2nd part tho? OK in that case, I will split this patch into 3. First patch for adding struct ravb_hw_info to driver data and replace driver data chip type with struct ravb_hw_info Second patch for changing ravb_private::num_tx_desc from int to unsigned int. Third patch for adding aligned_tx to struct ravb_hw_info. Regards, Biju
Hello! On 18.08.2021 9:29, Biju Das wrote: [...] >>>>>>> -----Original Message----- >>>>>>> On Mon, Aug 2, 2021 at 12:27 PM Biju Das >>>>>>> <biju.das.jz@bp.renesas.com> >>>>>>> wrote: >>>>>>>> The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L >>>>>>>> SoC are similar to the R-Car Ethernet AVB IP. With a few changes >>>>>>>> in the driver we can support both IPs. >>>>>>>> >>>>>>>> Currently a runtime decision based on the chip type is used to >>>>>>>> distinguish the HW differences between the SoC families. >>>>>>>> >>>>>>>> The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car >>>>>>>> Gen2 and RZ/G2L it is 2. For cases like this it is better to >>>>>>>> select the number of TX descriptors by using a structure with a >>>>>>>> value, rather than a runtime decision based on the chip type. >>>>>>>> >>>>>>>> This patch adds the num_tx_desc variable to struct ravb_hw_info >>>>>>>> and also replaces the driver data chip type with struct >>>>>>>> ravb_hw_info by moving chip type to it. >>>>>>>> >>>>>>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> >>>>>>>> Reviewed-by: Lad Prabhakar >>>>>>>> <prabhakar.mahadev-lad.rj@bp.renesas.com> >>>>>>> >>>>>>> Thanks for your patch! >>>>>>> >>>>>>>> --- a/drivers/net/ethernet/renesas/ravb.h >>>>>>>> +++ b/drivers/net/ethernet/renesas/ravb.h >>>>>>>> @@ -988,6 +988,11 @@ enum ravb_chip_id { >>>>>>>> RCAR_GEN3, >>>>>>>> }; >>>>>>>> >>>>>>>> +struct ravb_hw_info { >>>>>>>> + enum ravb_chip_id chip_id; >>>>>>>> + int num_tx_desc; >>>>>>> >>>>>>> Why not "unsigned int"? ... >>>>>>> This comment applies to a few more subsequent patches. >>>>>> >>>>>> To avoid signed and unsigned comparison warnings. >>>>>> >>>>>>> >>>>>>>> +}; >>>>>>>> + >>>>>>>> struct ravb_private { >>>>>>>> struct net_device *ndev; >>>>>>>> struct platform_device *pdev; @@ -1040,6 +1045,8 @@ >>>>>>>> struct ravb_private { >>>>>>>> unsigned txcidm:1; /* TX Clock Internal Delay >>>>> Mode >>>>>>> */ >>>>>>>> unsigned rgmii_override:1; /* Deprecated rgmii-*id >>>>> behavior >>>>>>> */ >>>>>>>> int num_tx_desc; /* TX descriptors per >>>> packet >>>>> */ >>>>>>> >>>>>>> ... oh, here's the original culprit. >>>>>> >>>>>> Exactly, this the reason. >>>>>> >>>>>> Do you want me to change this into unsigned int? Please let me know. >>>>> >>>>> Up to you (or the maintainer? ;-) >>>>> >>>>> For new fields (in the other patches), I would use unsigned for all >>>>> unsigned values. Signed values have more pitfalls related to >>>>> undefined behavior. >>>> >>>> Sergei, What is your thoughts here? Please let me know. >>> >>> Here is my plan. >>> >>> I will split this patch into two as Andrew suggested and >> >> If you mran changing the ravb_private::num_tx_desc to *unsigned*, it'll >> be a good cleanup. What's would be the 2nd part tho? > > OK in that case, I will split this patch into 3. > > First patch for adding struct ravb_hw_info to driver data and replace > driver data chip type with struct ravb_hw_info Couldn't this be a 2nd patch?.. > Second patch for changing ravb_private::num_tx_desc from int to unsigned int. ... and this one the 1st? > Third patch for adding aligned_tx to struct ravb_hw_info. > > Regards, > Biju MBR, Sergey
Hi Sergei, > -----Original Message----- > Subject: Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to > driver data > > Hello! > > On 18.08.2021 9:29, Biju Das wrote: > > [...] > >>>>>>> -----Original Message----- > >>>>>>> On Mon, Aug 2, 2021 at 12:27 PM Biju Das > >>>>>>> <biju.das.jz@bp.renesas.com> > >>>>>>> wrote: > >>>>>>>> The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L > >>>>>>>> SoC are similar to the R-Car Ethernet AVB IP. With a few > >>>>>>>> changes in the driver we can support both IPs. > >>>>>>>> > >>>>>>>> Currently a runtime decision based on the chip type is used to > >>>>>>>> distinguish the HW differences between the SoC families. > >>>>>>>> > >>>>>>>> The number of TX descriptors for R-Car Gen3 is 1 whereas on > >>>>>>>> R-Car > >>>>>>>> Gen2 and RZ/G2L it is 2. For cases like this it is better to > >>>>>>>> select the number of TX descriptors by using a structure with a > >>>>>>>> value, rather than a runtime decision based on the chip type. > >>>>>>>> > >>>>>>>> This patch adds the num_tx_desc variable to struct ravb_hw_info > >>>>>>>> and also replaces the driver data chip type with struct > >>>>>>>> ravb_hw_info by moving chip type to it. > >>>>>>>> > >>>>>>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > >>>>>>>> Reviewed-by: Lad Prabhakar > >>>>>>>> <prabhakar.mahadev-lad.rj@bp.renesas.com> > >>>>>>> > >>>>>>> Thanks for your patch! > >>>>>>> > >>>>>>>> --- a/drivers/net/ethernet/renesas/ravb.h > >>>>>>>> +++ b/drivers/net/ethernet/renesas/ravb.h > >>>>>>>> @@ -988,6 +988,11 @@ enum ravb_chip_id { > >>>>>>>> RCAR_GEN3, > >>>>>>>> }; > >>>>>>>> > >>>>>>>> +struct ravb_hw_info { > >>>>>>>> + enum ravb_chip_id chip_id; > >>>>>>>> + int num_tx_desc; > >>>>>>> > >>>>>>> Why not "unsigned int"? ... > >>>>>>> This comment applies to a few more subsequent patches. > >>>>>> > >>>>>> To avoid signed and unsigned comparison warnings. > >>>>>> > >>>>>>> > >>>>>>>> +}; > >>>>>>>> + > >>>>>>>> struct ravb_private { > >>>>>>>> struct net_device *ndev; > >>>>>>>> struct platform_device *pdev; @@ -1040,6 +1045,8 @@ > >>>>>>>> struct ravb_private { > >>>>>>>> unsigned txcidm:1; /* TX Clock Internal > Delay > >>>>> Mode > >>>>>>> */ > >>>>>>>> unsigned rgmii_override:1; /* Deprecated rgmii-*id > >>>>> behavior > >>>>>>> */ > >>>>>>>> int num_tx_desc; /* TX descriptors per > >>>> packet > >>>>> */ > >>>>>>> > >>>>>>> ... oh, here's the original culprit. > >>>>>> > >>>>>> Exactly, this the reason. > >>>>>> > >>>>>> Do you want me to change this into unsigned int? Please let me > know. > >>>>> > >>>>> Up to you (or the maintainer? ;-) > >>>>> > >>>>> For new fields (in the other patches), I would use unsigned for > >>>>> all unsigned values. Signed values have more pitfalls related to > >>>>> undefined behavior. > >>>> > >>>> Sergei, What is your thoughts here? Please let me know. > >>> > >>> Here is my plan. > >>> > >>> I will split this patch into two as Andrew suggested and > >> > >> If you mran changing the ravb_private::num_tx_desc to *unsigned*, > >> it'll be a good cleanup. What's would be the 2nd part tho? > > > > OK in that case, I will split this patch into 3. > > > > First patch for adding struct ravb_hw_info to driver data and replace > > driver data chip type with struct ravb_hw_info > > Couldn't this be a 2nd patch?.. > > > Second patch for changing ravb_private::num_tx_desc from int to unsigned > int. > > ... and this one the 1st? > > > Third patch for adding aligned_tx to struct ravb_hw_info. Sure. Will do. Cheers, Biju
diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h index 80e62ca2e3d3..cfb972c05b34 100644 --- a/drivers/net/ethernet/renesas/ravb.h +++ b/drivers/net/ethernet/renesas/ravb.h @@ -988,6 +988,11 @@ enum ravb_chip_id { RCAR_GEN3, }; +struct ravb_hw_info { + enum ravb_chip_id chip_id; + int num_tx_desc; +}; + struct ravb_private { struct net_device *ndev; struct platform_device *pdev; @@ -1040,6 +1045,8 @@ struct ravb_private { unsigned txcidm:1; /* TX Clock Internal Delay Mode */ unsigned rgmii_override:1; /* Deprecated rgmii-*id behavior */ int num_tx_desc; /* TX descriptors per packet */ + + const struct ravb_hw_info *info; }; static inline u32 ravb_read(struct net_device *ndev, enum ravb_reg reg) diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c index f4dfe9f71d06..ffbd224d8780 100644 --- a/drivers/net/ethernet/renesas/ravb_main.c +++ b/drivers/net/ethernet/renesas/ravb_main.c @@ -1924,12 +1924,22 @@ static int ravb_mdio_release(struct ravb_private *priv) return 0; } +static const struct ravb_hw_info ravb_gen3_hw_info = { + .chip_id = RCAR_GEN3, + .num_tx_desc = NUM_TX_DESC_GEN3, +}; + +static const struct ravb_hw_info ravb_gen2_hw_info = { + .chip_id = RCAR_GEN2, + .num_tx_desc = NUM_TX_DESC_GEN2, +}; + static const struct of_device_id ravb_match_table[] = { - { .compatible = "renesas,etheravb-r8a7790", .data = (void *)RCAR_GEN2 }, - { .compatible = "renesas,etheravb-r8a7794", .data = (void *)RCAR_GEN2 }, - { .compatible = "renesas,etheravb-rcar-gen2", .data = (void *)RCAR_GEN2 }, - { .compatible = "renesas,etheravb-r8a7795", .data = (void *)RCAR_GEN3 }, - { .compatible = "renesas,etheravb-rcar-gen3", .data = (void *)RCAR_GEN3 }, + { .compatible = "renesas,etheravb-r8a7790", .data = &ravb_gen2_hw_info }, + { .compatible = "renesas,etheravb-r8a7794", .data = &ravb_gen2_hw_info }, + { .compatible = "renesas,etheravb-rcar-gen2", .data = &ravb_gen2_hw_info }, + { .compatible = "renesas,etheravb-r8a7795", .data = &ravb_gen3_hw_info }, + { .compatible = "renesas,etheravb-rcar-gen3", .data = &ravb_gen3_hw_info }, { } }; MODULE_DEVICE_TABLE(of, ravb_match_table); @@ -2034,8 +2044,8 @@ static void ravb_set_delay_mode(struct net_device *ndev) static int ravb_probe(struct platform_device *pdev) { struct device_node *np = pdev->dev.of_node; + const struct ravb_hw_info *info; struct ravb_private *priv; - enum ravb_chip_id chip_id; struct net_device *ndev; int error, irq, q; struct resource *res; @@ -2058,9 +2068,9 @@ static int ravb_probe(struct platform_device *pdev) pm_runtime_enable(&pdev->dev); pm_runtime_get_sync(&pdev->dev); - chip_id = (enum ravb_chip_id)of_device_get_match_data(&pdev->dev); + info = of_device_get_match_data(&pdev->dev); - if (chip_id == RCAR_GEN3) + if (info->chip_id == RCAR_GEN3) irq = platform_get_irq_byname(pdev, "ch22"); else irq = platform_get_irq(pdev, 0); @@ -2073,6 +2083,7 @@ static int ravb_probe(struct platform_device *pdev) SET_NETDEV_DEV(ndev, &pdev->dev); priv = netdev_priv(ndev); + priv->info = info; priv->ndev = ndev; priv->pdev = pdev; priv->num_tx_ring[RAVB_BE] = BE_TX_RING_SIZE; @@ -2099,7 +2110,7 @@ static int ravb_probe(struct platform_device *pdev) priv->avb_link_active_low = of_property_read_bool(np, "renesas,ether-link-active-low"); - if (chip_id == RCAR_GEN3) { + if (info->chip_id == RCAR_GEN3) { irq = platform_get_irq_byname(pdev, "ch24"); if (irq < 0) { error = irq; @@ -2124,7 +2135,7 @@ static int ravb_probe(struct platform_device *pdev) } } - priv->chip_id = chip_id; + priv->chip_id = info->chip_id; priv->clk = devm_clk_get(&pdev->dev, NULL); if (IS_ERR(priv->clk)) { @@ -2142,8 +2153,7 @@ static int ravb_probe(struct platform_device *pdev) ndev->max_mtu = 2048 - (ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN); ndev->min_mtu = ETH_MIN_MTU; - priv->num_tx_desc = chip_id == RCAR_GEN2 ? - NUM_TX_DESC_GEN2 : NUM_TX_DESC_GEN3; + priv->num_tx_desc = info->num_tx_desc; /* Set function */ ndev->netdev_ops = &ravb_netdev_ops; @@ -2184,7 +2194,7 @@ static int ravb_probe(struct platform_device *pdev) INIT_LIST_HEAD(&priv->ts_skb_list); /* Initialise PTP Clock driver */ - if (chip_id != RCAR_GEN2) + if (info->chip_id != RCAR_GEN2) ravb_ptp_init(ndev, pdev); /* Debug message level */ @@ -2232,7 +2242,7 @@ static int ravb_probe(struct platform_device *pdev) priv->desc_bat_dma); /* Stop PTP Clock driver */ - if (chip_id != RCAR_GEN2) + if (info->chip_id != RCAR_GEN2) ravb_ptp_stop(ndev); out_disable_refclk: clk_disable_unprepare(priv->refclk);