netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 19/23] net: ethernet: freescale: make UCC_GETH explicitly depend on PPC32
       [not found] ` <20191025124058.22580-1-linux@rasmusvillemoes.dk>
@ 2019-10-25 12:40   ` Rasmus Villemoes
  2019-10-30 10:55     ` Christophe Leroy
  0 siblings, 1 reply; 13+ messages in thread
From: Rasmus Villemoes @ 2019-10-25 12:40 UTC (permalink / raw)
  To: Qiang Zhao, Li Yang, Christophe Leroy
  Cc: linuxppc-dev, linux-arm-kernel, linux-kernel, Scott Wood,
	Valentin Longchamp, Rasmus Villemoes, netdev

Currently, QUICC_ENGINE depends on PPC32, so this in itself does not
change anything. In order to allow removing the PPC32 dependency from
QUICC_ENGINE and avoid allmodconfig build failures, add this explicit
dependency.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/net/ethernet/freescale/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/freescale/Kconfig b/drivers/net/ethernet/freescale/Kconfig
index 6a7e8993119f..97d27c7740d4 100644
--- a/drivers/net/ethernet/freescale/Kconfig
+++ b/drivers/net/ethernet/freescale/Kconfig
@@ -75,6 +75,7 @@ config FSL_XGMAC_MDIO
 config UCC_GETH
 	tristate "Freescale QE Gigabit Ethernet"
 	depends on QUICC_ENGINE
+	depends on PPC32
 	select FSL_PQ_MDIO
 	select PHYLIB
 	---help---
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 19/23] net: ethernet: freescale: make UCC_GETH explicitly depend on PPC32
  2019-10-25 12:40   ` [PATCH v2 19/23] net: ethernet: freescale: make UCC_GETH explicitly depend on PPC32 Rasmus Villemoes
@ 2019-10-30 10:55     ` Christophe Leroy
  0 siblings, 0 replies; 13+ messages in thread
From: Christophe Leroy @ 2019-10-30 10:55 UTC (permalink / raw)
  To: Rasmus Villemoes, Qiang Zhao, Li Yang
  Cc: linuxppc-dev, linux-arm-kernel, linux-kernel, Scott Wood,
	Valentin Longchamp, netdev



Le 25/10/2019 à 14:40, Rasmus Villemoes a écrit :
> Currently, QUICC_ENGINE depends on PPC32, so this in itself does not
> change anything. In order to allow removing the PPC32 dependency from
> QUICC_ENGINE and avoid allmodconfig build failures, add this explicit
> dependency.
> 
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
>   drivers/net/ethernet/freescale/Kconfig | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/ethernet/freescale/Kconfig b/drivers/net/ethernet/freescale/Kconfig
> index 6a7e8993119f..97d27c7740d4 100644
> --- a/drivers/net/ethernet/freescale/Kconfig
> +++ b/drivers/net/ethernet/freescale/Kconfig
> @@ -75,6 +75,7 @@ config FSL_XGMAC_MDIO
>   config UCC_GETH
>   	tristate "Freescale QE Gigabit Ethernet"
>   	depends on QUICC_ENGINE
> +	depends on PPC32

I think it would be more obvious to have:
	depends on QUICC_ENGINE && PPC32

Christophe

>   	select FSL_PQ_MDIO
>   	select PHYLIB
>   	---help---
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v3 03/36] soc: fsl: qe: rename qe_(clr/set/clrset)bit* helpers
       [not found] ` <20191101124210.14510-1-linux@rasmusvillemoes.dk>
@ 2019-11-01 12:41   ` Rasmus Villemoes
  2019-11-01 12:42   ` [PATCH v3 34/36] net: ethernet: freescale: make UCC_GETH explicitly depend on PPC32 Rasmus Villemoes
  2019-11-01 12:42   ` [PATCH v3 35/36] net/wan: make FSL_UCC_HDLC " Rasmus Villemoes
  2 siblings, 0 replies; 13+ messages in thread
From: Rasmus Villemoes @ 2019-11-01 12:41 UTC (permalink / raw)
  To: Qiang Zhao, Li Yang, Christophe Leroy
  Cc: linuxppc-dev, linux-arm-kernel, linux-kernel, Scott Wood,
	Rasmus Villemoes, netdev

Make it clear that these operate on big-endian registers (i.e. use the
iowrite*be primitives) before we introduce more uses of them and allow
the QE drivers to be built for platforms other than ppc32.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/net/wan/fsl_ucc_hdlc.c |  4 ++--
 drivers/soc/fsl/qe/ucc.c       | 10 +++++-----
 include/soc/fsl/qe/qe.h        | 18 +++++++++---------
 3 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index ca0f3be2b6bf..ce6af7d5380f 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -623,8 +623,8 @@ static int ucc_hdlc_poll(struct napi_struct *napi, int budget)
 
 	if (howmany < budget) {
 		napi_complete_done(napi, howmany);
-		qe_setbits32(priv->uccf->p_uccm,
-			     (UCCE_HDLC_RX_EVENTS | UCCE_HDLC_TX_EVENTS) << 16);
+		qe_setbits_be32(priv->uccf->p_uccm,
+				(UCCE_HDLC_RX_EVENTS | UCCE_HDLC_TX_EVENTS) << 16);
 	}
 
 	return howmany;
diff --git a/drivers/soc/fsl/qe/ucc.c b/drivers/soc/fsl/qe/ucc.c
index 024d239ac1e1..ae9f2cf560cb 100644
--- a/drivers/soc/fsl/qe/ucc.c
+++ b/drivers/soc/fsl/qe/ucc.c
@@ -540,8 +540,8 @@ int ucc_set_tdm_rxtx_clk(u32 tdm_num, enum qe_clock clock,
 	cmxs1cr = (tdm_num < 4) ? &qe_mux_reg->cmxsi1cr_l :
 				  &qe_mux_reg->cmxsi1cr_h;
 
-	qe_clrsetbits32(cmxs1cr, QE_CMXUCR_TX_CLK_SRC_MASK << shift,
-			clock_bits << shift);
+	qe_clrsetbits_be32(cmxs1cr, QE_CMXUCR_TX_CLK_SRC_MASK << shift,
+			   clock_bits << shift);
 
 	return 0;
 }
@@ -650,9 +650,9 @@ int ucc_set_tdm_rxtx_sync(u32 tdm_num, enum qe_clock clock,
 
 	shift = ucc_get_tdm_sync_shift(mode, tdm_num);
 
-	qe_clrsetbits32(&qe_mux_reg->cmxsi1syr,
-			QE_CMXUCR_TX_CLK_SRC_MASK << shift,
-			source << shift);
+	qe_clrsetbits_be32(&qe_mux_reg->cmxsi1syr,
+			   QE_CMXUCR_TX_CLK_SRC_MASK << shift,
+			   source << shift);
 
 	return 0;
 }
diff --git a/include/soc/fsl/qe/qe.h b/include/soc/fsl/qe/qe.h
index c1036d16ed03..a1aa4eb28f0c 100644
--- a/include/soc/fsl/qe/qe.h
+++ b/include/soc/fsl/qe/qe.h
@@ -241,20 +241,20 @@ static inline int qe_alive_during_sleep(void)
 #define qe_muram_offset cpm_muram_offset
 #define qe_muram_dma cpm_muram_dma
 
-#define qe_setbits32(_addr, _v) iowrite32be(ioread32be(_addr) |  (_v), (_addr))
-#define qe_clrbits32(_addr, _v) iowrite32be(ioread32be(_addr) & ~(_v), (_addr))
+#define qe_setbits_be32(_addr, _v) iowrite32be(ioread32be(_addr) |  (_v), (_addr))
+#define qe_clrbits_be32(_addr, _v) iowrite32be(ioread32be(_addr) & ~(_v), (_addr))
 
-#define qe_setbits16(_addr, _v) iowrite16be(ioread16be(_addr) |  (_v), (_addr))
-#define qe_clrbits16(_addr, _v) iowrite16be(ioread16be(_addr) & ~(_v), (_addr))
+#define qe_setbits_be16(_addr, _v) iowrite16be(ioread16be(_addr) |  (_v), (_addr))
+#define qe_clrbits_be16(_addr, _v) iowrite16be(ioread16be(_addr) & ~(_v), (_addr))
 
-#define qe_setbits8(_addr, _v) iowrite8(ioread8(_addr) |  (_v), (_addr))
-#define qe_clrbits8(_addr, _v) iowrite8(ioread8(_addr) & ~(_v), (_addr))
+#define qe_setbits_8(_addr, _v) iowrite8(ioread8(_addr) |  (_v), (_addr))
+#define qe_clrbits_8(_addr, _v) iowrite8(ioread8(_addr) & ~(_v), (_addr))
 
-#define qe_clrsetbits32(addr, clear, set) \
+#define qe_clrsetbits_be32(addr, clear, set) \
 	iowrite32be((ioread32be(addr) & ~(clear)) | (set), (addr))
-#define qe_clrsetbits16(addr, clear, set) \
+#define qe_clrsetbits_be16(addr, clear, set) \
 	iowrite16be((ioread16be(addr) & ~(clear)) | (set), (addr))
-#define qe_clrsetbits8(addr, clear, set) \
+#define qe_clrsetbits_8(addr, clear, set) \
 	iowrite8((ioread8(addr) & ~(clear)) | (set), (addr))
 
 /* Structure that defines QE firmware binary files.
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 34/36] net: ethernet: freescale: make UCC_GETH explicitly depend on PPC32
       [not found] ` <20191101124210.14510-1-linux@rasmusvillemoes.dk>
  2019-11-01 12:41   ` [PATCH v3 03/36] soc: fsl: qe: rename qe_(clr/set/clrset)bit* helpers Rasmus Villemoes
@ 2019-11-01 12:42   ` Rasmus Villemoes
  2019-11-01 12:42   ` [PATCH v3 35/36] net/wan: make FSL_UCC_HDLC " Rasmus Villemoes
  2 siblings, 0 replies; 13+ messages in thread
From: Rasmus Villemoes @ 2019-11-01 12:42 UTC (permalink / raw)
  To: Qiang Zhao, Li Yang, Christophe Leroy
  Cc: linuxppc-dev, linux-arm-kernel, linux-kernel, Scott Wood,
	Rasmus Villemoes, netdev

Currently, QUICC_ENGINE depends on PPC32, so this in itself does not
change anything. In order to allow removing the PPC32 dependency from
QUICC_ENGINE and avoid allmodconfig build failures, add this explicit
dependency.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/net/ethernet/freescale/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/Kconfig b/drivers/net/ethernet/freescale/Kconfig
index 6a7e8993119f..2bd7ace0a953 100644
--- a/drivers/net/ethernet/freescale/Kconfig
+++ b/drivers/net/ethernet/freescale/Kconfig
@@ -74,7 +74,7 @@ config FSL_XGMAC_MDIO
 
 config UCC_GETH
 	tristate "Freescale QE Gigabit Ethernet"
-	depends on QUICC_ENGINE
+	depends on QUICC_ENGINE && PPC32
 	select FSL_PQ_MDIO
 	select PHYLIB
 	---help---
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 35/36] net/wan: make FSL_UCC_HDLC explicitly depend on PPC32
       [not found] ` <20191101124210.14510-1-linux@rasmusvillemoes.dk>
  2019-11-01 12:41   ` [PATCH v3 03/36] soc: fsl: qe: rename qe_(clr/set/clrset)bit* helpers Rasmus Villemoes
  2019-11-01 12:42   ` [PATCH v3 34/36] net: ethernet: freescale: make UCC_GETH explicitly depend on PPC32 Rasmus Villemoes
@ 2019-11-01 12:42   ` Rasmus Villemoes
  2019-11-01 16:29     ` Christophe Leroy
  2 siblings, 1 reply; 13+ messages in thread
From: Rasmus Villemoes @ 2019-11-01 12:42 UTC (permalink / raw)
  To: Qiang Zhao, Li Yang, Christophe Leroy
  Cc: linuxppc-dev, linux-arm-kernel, linux-kernel, Scott Wood,
	Rasmus Villemoes, netdev

Currently, FSL_UCC_HDLC depends on QUICC_ENGINE, which in turn depends
on PPC32. As preparation for removing the latter and thus allowing the
core QE code to be built for other architectures, make FSL_UCC_HDLC
explicitly depend on PPC32.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/net/wan/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wan/Kconfig b/drivers/net/wan/Kconfig
index dd1a147f2971..78785d790bcc 100644
--- a/drivers/net/wan/Kconfig
+++ b/drivers/net/wan/Kconfig
@@ -270,7 +270,7 @@ config FARSYNC
 config FSL_UCC_HDLC
 	tristate "Freescale QUICC Engine HDLC support"
 	depends on HDLC
-	depends on QUICC_ENGINE
+	depends on QUICC_ENGINE && PPC32
 	help
 	  Driver for Freescale QUICC Engine HDLC controller. The driver
 	  supports HDLC in NMSI and TDM mode.
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 35/36] net/wan: make FSL_UCC_HDLC explicitly depend on PPC32
  2019-11-01 12:42   ` [PATCH v3 35/36] net/wan: make FSL_UCC_HDLC " Rasmus Villemoes
@ 2019-11-01 16:29     ` Christophe Leroy
  2019-11-01 22:31       ` Leo Li
  0 siblings, 1 reply; 13+ messages in thread
From: Christophe Leroy @ 2019-11-01 16:29 UTC (permalink / raw)
  To: Rasmus Villemoes, Qiang Zhao, Li Yang
  Cc: linuxppc-dev, linux-arm-kernel, linux-kernel, Scott Wood, netdev



Le 01/11/2019 à 13:42, Rasmus Villemoes a écrit :
> Currently, FSL_UCC_HDLC depends on QUICC_ENGINE, which in turn depends
> on PPC32. As preparation for removing the latter and thus allowing the
> core QE code to be built for other architectures, make FSL_UCC_HDLC
> explicitly depend on PPC32.

Is that really powerpc specific ? Can't the ARM QE perform HDLC on UCC ?

Christophe

> 
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
>   drivers/net/wan/Kconfig | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wan/Kconfig b/drivers/net/wan/Kconfig
> index dd1a147f2971..78785d790bcc 100644
> --- a/drivers/net/wan/Kconfig
> +++ b/drivers/net/wan/Kconfig
> @@ -270,7 +270,7 @@ config FARSYNC
>   config FSL_UCC_HDLC
>   	tristate "Freescale QUICC Engine HDLC support"
>   	depends on HDLC
> -	depends on QUICC_ENGINE
> +	depends on QUICC_ENGINE && PPC32
>   	help
>   	  Driver for Freescale QUICC Engine HDLC controller. The driver
>   	  supports HDLC in NMSI and TDM mode.
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH v3 35/36] net/wan: make FSL_UCC_HDLC explicitly depend on PPC32
  2019-11-01 16:29     ` Christophe Leroy
@ 2019-11-01 22:31       ` Leo Li
  2019-11-04  8:38         ` Rasmus Villemoes
  0 siblings, 1 reply; 13+ messages in thread
From: Leo Li @ 2019-11-01 22:31 UTC (permalink / raw)
  To: Christophe Leroy, Rasmus Villemoes, Qiang Zhao
  Cc: linuxppc-dev, linux-arm-kernel, linux-kernel, Scott Wood, netdev



> -----Original Message-----
> From: Christophe Leroy <christophe.leroy@c-s.fr>
> Sent: Friday, November 1, 2019 11:30 AM
> To: Rasmus Villemoes <linux@rasmusvillemoes.dk>; Qiang Zhao
> <qiang.zhao@nxp.com>; Leo Li <leoyang.li@nxp.com>
> Cc: linuxppc-dev@lists.ozlabs.org; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org; Scott Wood <oss@buserror.net>;
> netdev@vger.kernel.org
> Subject: Re: [PATCH v3 35/36] net/wan: make FSL_UCC_HDLC explicitly
> depend on PPC32
> 
> 
> 
> Le 01/11/2019 à 13:42, Rasmus Villemoes a écrit :
> > Currently, FSL_UCC_HDLC depends on QUICC_ENGINE, which in turn
> depends
> > on PPC32. As preparation for removing the latter and thus allowing the
> > core QE code to be built for other architectures, make FSL_UCC_HDLC
> > explicitly depend on PPC32.
> 
> Is that really powerpc specific ? Can't the ARM QE perform HDLC on UCC ?

No.  Actually the HDLC and TDM are the major reason to integrate a QE on the ARM based Layerscape SoCs.

Since Rasmus doesn't have the hardware to test this feature Qiang Zhao probably can help verify the functionality of TDM and we can drop this patch.

Regards,
Leo

> 
> Christophe
> 
> >
> > Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> > ---
> >   drivers/net/wan/Kconfig | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/wan/Kconfig b/drivers/net/wan/Kconfig index
> > dd1a147f2971..78785d790bcc 100644
> > --- a/drivers/net/wan/Kconfig
> > +++ b/drivers/net/wan/Kconfig
> > @@ -270,7 +270,7 @@ config FARSYNC
> >   config FSL_UCC_HDLC
> >   	tristate "Freescale QUICC Engine HDLC support"
> >   	depends on HDLC
> > -	depends on QUICC_ENGINE
> > +	depends on QUICC_ENGINE && PPC32
> >   	help
> >   	  Driver for Freescale QUICC Engine HDLC controller. The driver
> >   	  supports HDLC in NMSI and TDM mode.
> >

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 35/36] net/wan: make FSL_UCC_HDLC explicitly depend on PPC32
  2019-11-01 22:31       ` Leo Li
@ 2019-11-04  8:38         ` Rasmus Villemoes
  2019-11-04 20:56           ` Li Yang
  2019-11-05  6:16           ` Qiang Zhao
  0 siblings, 2 replies; 13+ messages in thread
From: Rasmus Villemoes @ 2019-11-04  8:38 UTC (permalink / raw)
  To: Leo Li, Christophe Leroy, Qiang Zhao
  Cc: linuxppc-dev, linux-arm-kernel, linux-kernel, Scott Wood, netdev

On 01/11/2019 23.31, Leo Li wrote:
> 
> 
>> -----Original Message-----
>> From: Christophe Leroy <christophe.leroy@c-s.fr>
>> Sent: Friday, November 1, 2019 11:30 AM
>> To: Rasmus Villemoes <linux@rasmusvillemoes.dk>; Qiang Zhao
>> <qiang.zhao@nxp.com>; Leo Li <leoyang.li@nxp.com>
>> Cc: linuxppc-dev@lists.ozlabs.org; linux-arm-kernel@lists.infradead.org;
>> linux-kernel@vger.kernel.org; Scott Wood <oss@buserror.net>;
>> netdev@vger.kernel.org
>> Subject: Re: [PATCH v3 35/36] net/wan: make FSL_UCC_HDLC explicitly
>> depend on PPC32
>>
>>
>>
>> Le 01/11/2019 à 13:42, Rasmus Villemoes a écrit :
>>> Currently, FSL_UCC_HDLC depends on QUICC_ENGINE, which in turn
>> depends
>>> on PPC32. As preparation for removing the latter and thus allowing the
>>> core QE code to be built for other architectures, make FSL_UCC_HDLC
>>> explicitly depend on PPC32.
>>
>> Is that really powerpc specific ? Can't the ARM QE perform HDLC on UCC ?

I think the driver would build on ARM. Whether it works I don't know. I
know it does not build on 64 bit hosts (see kbuild report for v2,23/23).

> No.  Actually the HDLC and TDM are the major reason to integrate a QE on the ARM based Layerscape SoCs.

[citation needed].

> Since Rasmus doesn't have the hardware to test this feature Qiang Zhao probably can help verify the functionality of TDM and we can drop this patch.

No, this patch cannot be dropped. Please see the kbuild complaints for
v2,23/23 about use of IS_ERR_VALUE on not-sizeof(long) entities. I see
kbuild has complained about the same thing for v3 since apparently the
same thing appears in ucc_slow.c. So I'll fix that.

Moreover, as you say and know, I do not have the hardware to test it, so
I'm not going to even attempt to fix up fsl_ucc_hdlc.c. If Qiang Zhao or
somebody else can verify that it works just fine on ARM and fixes the
allmodconfig problem(s), he/she is more than welcome to sign off on a
patch that removes the CONFIG_PPC32 dependency or replaces it with
something else.

Rasmus

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 35/36] net/wan: make FSL_UCC_HDLC explicitly depend on PPC32
  2019-11-04  8:38         ` Rasmus Villemoes
@ 2019-11-04 20:56           ` Li Yang
  2019-11-05 22:46             ` Rasmus Villemoes
  2019-11-05  6:16           ` Qiang Zhao
  1 sibling, 1 reply; 13+ messages in thread
From: Li Yang @ 2019-11-04 20:56 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Christophe Leroy, Qiang Zhao, linuxppc-dev, linux-arm-kernel,
	linux-kernel, Scott Wood, netdev

On Mon, Nov 4, 2019 at 2:39 AM Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
>
> On 01/11/2019 23.31, Leo Li wrote:
> >
> >
> >> -----Original Message-----
> >> From: Christophe Leroy <christophe.leroy@c-s.fr>
> >> Sent: Friday, November 1, 2019 11:30 AM
> >> To: Rasmus Villemoes <linux@rasmusvillemoes.dk>; Qiang Zhao
> >> <qiang.zhao@nxp.com>; Leo Li <leoyang.li@nxp.com>
> >> Cc: linuxppc-dev@lists.ozlabs.org; linux-arm-kernel@lists.infradead.org;
> >> linux-kernel@vger.kernel.org; Scott Wood <oss@buserror.net>;
> >> netdev@vger.kernel.org
> >> Subject: Re: [PATCH v3 35/36] net/wan: make FSL_UCC_HDLC explicitly
> >> depend on PPC32
> >>
> >>
> >>
> >> Le 01/11/2019 à 13:42, Rasmus Villemoes a écrit :
> >>> Currently, FSL_UCC_HDLC depends on QUICC_ENGINE, which in turn
> >> depends
> >>> on PPC32. As preparation for removing the latter and thus allowing the
> >>> core QE code to be built for other architectures, make FSL_UCC_HDLC
> >>> explicitly depend on PPC32.
> >>
> >> Is that really powerpc specific ? Can't the ARM QE perform HDLC on UCC ?
>
> I think the driver would build on ARM. Whether it works I don't know. I
> know it does not build on 64 bit hosts (see kbuild report for v2,23/23).

The problem for arm64 can be easy to fix.  Actually I don't think it
is neccessarily to be compiled on all architectures except powerpc,
arm and arm64.  I am surprised that you made the core QE code compile
for all architecture(although still have some kbuild warnings)

>
> > No.  Actually the HDLC and TDM are the major reason to integrate a QE on the ARM based Layerscape SoCs.
>
> [citation needed].

I got this message from our marketing team.  Also it is reflected on
marketing materials like
https://www.nxp.com/products/processors-and-microcontrollers/arm-processors/layerscape-communication-process/qoriq-layerscape-1043a-and-1023a-multicore-communications-processors:LS1043A

"The QorIQ LS1043A ... integrated QUICC Engine® for legacy glue-less
HDLC, TDM or Profibus support."

>
> > Since Rasmus doesn't have the hardware to test this feature Qiang Zhao probably can help verify the functionality of TDM and we can drop this patch.
>
> No, this patch cannot be dropped. Please see the kbuild complaints for
> v2,23/23 about use of IS_ERR_VALUE on not-sizeof(long) entities. I see
> kbuild has complained about the same thing for v3 since apparently the
> same thing appears in ucc_slow.c. So I'll fix that.

When I made this comment I didn't notice you have removed all the
architectural dependencies for CONFIG_QUICC_ENGINE.  If the
QUICC_ENGINE is only buidable on powerpc, arm and arm64, this change
will not be needed.

BTW, I'm not sure if it is a good idea to make it selectable on these
unrelavent architectures.  Real architectural dependencies and
COMPILE_TEST dependency will be better if we really want to test the
buildability on other platforms.

Regards,
Leo

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH v3 35/36] net/wan: make FSL_UCC_HDLC explicitly depend on PPC32
  2019-11-04  8:38         ` Rasmus Villemoes
  2019-11-04 20:56           ` Li Yang
@ 2019-11-05  6:16           ` Qiang Zhao
  2019-11-06  7:56             ` Rasmus Villemoes
  1 sibling, 1 reply; 13+ messages in thread
From: Qiang Zhao @ 2019-11-05  6:16 UTC (permalink / raw)
  To: Rasmus Villemoes, Leo Li, Christophe Leroy
  Cc: linuxppc-dev, linux-arm-kernel, linux-kernel, Scott Wood, netdev

On 01/11/2019 23:31, Rasmus Villemoes wrote :


> -----Original Message-----
> From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Sent: 2019年11月4日 16:38
> To: Leo Li <leoyang.li@nxp.com>; Christophe Leroy <christophe.leroy@c-s.fr>;
> Qiang Zhao <qiang.zhao@nxp.com>
> Cc: linuxppc-dev@lists.ozlabs.org; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org; Scott Wood <oss@buserror.net>;
> netdev@vger.kernel.org
> Subject: Re: [PATCH v3 35/36] net/wan: make FSL_UCC_HDLC explicitly depend
> on PPC32
> 
> On 01/11/2019 23.31, Leo Li wrote:
> >
> >
> >> -----Original Message-----
> >> From: Christophe Leroy <christophe.leroy@c-s.fr>
> >> Sent: Friday, November 1, 2019 11:30 AM
> >> To: Rasmus Villemoes <linux@rasmusvillemoes.dk>; Qiang Zhao
> >> <qiang.zhao@nxp.com>; Leo Li <leoyang.li@nxp.com>
> >> Cc: linuxppc-dev@lists.ozlabs.org;
> >> linux-arm-kernel@lists.infradead.org;
> >> linux-kernel@vger.kernel.org; Scott Wood <oss@buserror.net>;
> >> netdev@vger.kernel.org
> >> Subject: Re: [PATCH v3 35/36] net/wan: make FSL_UCC_HDLC explicitly
> >> depend on PPC32
> >>
> >>
> >>
> >> Le 01/11/2019 à 13:42, Rasmus Villemoes a écrit :
> >>> Currently, FSL_UCC_HDLC depends on QUICC_ENGINE, which in turn
> >> depends
> >>> on PPC32. As preparation for removing the latter and thus allowing
> >>> the core QE code to be built for other architectures, make
> >>> FSL_UCC_HDLC explicitly depend on PPC32.
> >>
> >> Is that really powerpc specific ? Can't the ARM QE perform HDLC on UCC ?
> 
> I think the driver would build on ARM. Whether it works I don't know. I know it
> does not build on 64 bit hosts (see kbuild report for v2,23/23).
> 
> > No.  Actually the HDLC and TDM are the major reason to integrate a QE on
> the ARM based Layerscape SoCs.
> 
> [citation needed].
> 
> > Since Rasmus doesn't have the hardware to test this feature Qiang Zhao
> probably can help verify the functionality of TDM and we can drop this patch.
> 
> No, this patch cannot be dropped. Please see the kbuild complaints for
> v2,23/23 about use of IS_ERR_VALUE on not-sizeof(long) entities. I see kbuild
> has complained about the same thing for v3 since apparently the same thing
> appears in ucc_slow.c. So I'll fix that.
> 
> Moreover, as you say and know, I do not have the hardware to test it, so I'm
> not going to even attempt to fix up fsl_ucc_hdlc.c. If Qiang Zhao or somebody
> else can verify that it works just fine on ARM and fixes the allmodconfig
> problem(s), he/she is more than welcome to sign off on a patch that removes
> the CONFIG_PPC32 dependency or replaces it with something else.
> 

I tested your v3 patches on ls1043ardb which is arm64 for fsl_ucc_hdlc, it can work,
Only it will put a compile warning, I also made a patch to fix it.
I can send a patch to remove PPC32 dependency when I send my patch to support ARM64.
Or I add my patch in your patchset.

Best Regards
Qiang Zhao

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 35/36] net/wan: make FSL_UCC_HDLC explicitly depend on PPC32
  2019-11-04 20:56           ` Li Yang
@ 2019-11-05 22:46             ` Rasmus Villemoes
  2019-11-05 23:46               ` Li Yang
  0 siblings, 1 reply; 13+ messages in thread
From: Rasmus Villemoes @ 2019-11-05 22:46 UTC (permalink / raw)
  To: Li Yang
  Cc: Christophe Leroy, Qiang Zhao, linuxppc-dev, linux-arm-kernel,
	linux-kernel, Scott Wood, netdev

On 04/11/2019 21.56, Li Yang wrote:

>> No, this patch cannot be dropped. Please see the kbuild complaints for
>> v2,23/23 about use of IS_ERR_VALUE on not-sizeof(long) entities. I see
>> kbuild has complained about the same thing for v3 since apparently the
>> same thing appears in ucc_slow.c. So I'll fix that.
> 
> When I made this comment I didn't notice you have removed all the
> architectural dependencies for CONFIG_QUICC_ENGINE.  If the
> QUICC_ENGINE is only buidable on powerpc, arm and arm64, this change
> will not be needed.
> 
> BTW, I'm not sure if it is a good idea to make it selectable on these
> unrelavent architectures.  Real architectural dependencies and
> COMPILE_TEST dependency will be better if we really want to test the
> buildability on other platforms.

Well, making QUICC_ENGINE depend on PPC32 || ARM would certainly make
things easier for me. Once you include ARM64 or any other 64 bit
architecture the buildbot complaints start rolling in from the
IS_ERR_VALUEs. And ARM64 should be supported as well, so there really
isn't much difference between dropping all arch restrictions and listing
the relevant archs in the Kconfig dependencies.

Rasmus



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 35/36] net/wan: make FSL_UCC_HDLC explicitly depend on PPC32
  2019-11-05 22:46             ` Rasmus Villemoes
@ 2019-11-05 23:46               ` Li Yang
  0 siblings, 0 replies; 13+ messages in thread
From: Li Yang @ 2019-11-05 23:46 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Christophe Leroy, Qiang Zhao, linuxppc-dev, linux-arm-kernel,
	linux-kernel, Scott Wood, netdev

On Tue, Nov 5, 2019 at 4:47 PM Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
>
> On 04/11/2019 21.56, Li Yang wrote:
>
> >> No, this patch cannot be dropped. Please see the kbuild complaints for
> >> v2,23/23 about use of IS_ERR_VALUE on not-sizeof(long) entities. I see
> >> kbuild has complained about the same thing for v3 since apparently the
> >> same thing appears in ucc_slow.c. So I'll fix that.
> >
> > When I made this comment I didn't notice you have removed all the
> > architectural dependencies for CONFIG_QUICC_ENGINE.  If the
> > QUICC_ENGINE is only buidable on powerpc, arm and arm64, this change
> > will not be needed.
> >
> > BTW, I'm not sure if it is a good idea to make it selectable on these
> > unrelavent architectures.  Real architectural dependencies and
> > COMPILE_TEST dependency will be better if we really want to test the
> > buildability on other platforms.
>
> Well, making QUICC_ENGINE depend on PPC32 || ARM would certainly make
> things easier for me. Once you include ARM64 or any other 64 bit
> architecture the buildbot complaints start rolling in from the
> IS_ERR_VALUEs. And ARM64 should be supported as well, so there really
> isn't much difference between dropping all arch restrictions and listing
> the relevant archs in the Kconfig dependencies.

I agree that it will be good to have the driver compile for all
architectures for compile test.  But list all the relevant
architectures and CONFIG_COMPILE_TEST as dependencies will make it not
really selected for these irrelevant architectures in real system.

Regards,
Leo

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 35/36] net/wan: make FSL_UCC_HDLC explicitly depend on PPC32
  2019-11-05  6:16           ` Qiang Zhao
@ 2019-11-06  7:56             ` Rasmus Villemoes
  0 siblings, 0 replies; 13+ messages in thread
From: Rasmus Villemoes @ 2019-11-06  7:56 UTC (permalink / raw)
  To: Qiang Zhao, Leo Li, Christophe Leroy
  Cc: linuxppc-dev, linux-arm-kernel, linux-kernel, Scott Wood, netdev

On 05/11/2019 07.16, Qiang Zhao wrote:

> 
> I tested your v3 patches on ls1043ardb which is arm64 for fsl_ucc_hdlc, it can work,
> Only it will put a compile warning, I also made a patch to fix it.
> I can send a patch to remove PPC32 dependency when I send my patch to support ARM64.
> Or I add my patch in your patchset.

Please send your patch (without whatever Kconfig hunk you needed to add)
with a proper changelog etc. If it looks reasonable (to me, reviewers of
the whole thing obviously also need to agree), I'll include it in my
series and drop adding the PPC32 addition to FSL_UCC_HDLC. Otherwise
I'll keep that and then you can later drop the PPC32 dependency.

Rasmus


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2019-11-06  7:56 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20191018125234.21825-1-linux@rasmusvillemoes.dk>
     [not found] ` <20191025124058.22580-1-linux@rasmusvillemoes.dk>
2019-10-25 12:40   ` [PATCH v2 19/23] net: ethernet: freescale: make UCC_GETH explicitly depend on PPC32 Rasmus Villemoes
2019-10-30 10:55     ` Christophe Leroy
     [not found] ` <20191101124210.14510-1-linux@rasmusvillemoes.dk>
2019-11-01 12:41   ` [PATCH v3 03/36] soc: fsl: qe: rename qe_(clr/set/clrset)bit* helpers Rasmus Villemoes
2019-11-01 12:42   ` [PATCH v3 34/36] net: ethernet: freescale: make UCC_GETH explicitly depend on PPC32 Rasmus Villemoes
2019-11-01 12:42   ` [PATCH v3 35/36] net/wan: make FSL_UCC_HDLC " Rasmus Villemoes
2019-11-01 16:29     ` Christophe Leroy
2019-11-01 22:31       ` Leo Li
2019-11-04  8:38         ` Rasmus Villemoes
2019-11-04 20:56           ` Li Yang
2019-11-05 22:46             ` Rasmus Villemoes
2019-11-05 23:46               ` Li Yang
2019-11-05  6:16           ` Qiang Zhao
2019-11-06  7:56             ` Rasmus Villemoes

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).