Re: [PATCH v3 35/36] net/wan: make FSL_UCC_HDLC explicitly depend on PPC32

[Rasmus Villemoes]

On 01/11/2019 23.31, Leo Li wrote:
> 
> 
>> -----Original Message-----
>> From: Christophe Leroy <christophe.leroy@xxxxxx>
>> Sent: Friday, November 1, 2019 11:30 AM
>> To: Rasmus Villemoes <linux@xxxxxxxxxxxxxxxxxx>; Qiang Zhao
>> <qiang.zhao@xxxxxxx>; Leo Li <leoyang.li@xxxxxxx>
>> Cc: linuxppc-dev@xxxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
>> linux-kernel@xxxxxxxxxxxxxxx; Scott Wood <oss@xxxxxxxxxxxx>;
>> netdev@xxxxxxxxxxxxxxx
>> 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