Re: [PATCH] net: macb: Add big endian CPU support
On 02/24/2015 08:39 AM, Arun Chandran wrote:
> This patch converts all __raw_readl and __raw_writel function calls
> to their corresponding readl_relaxed and writel_relaxed variants.
>
> It also tells the driver to set ahb_endian_swp_mgmt_en bit in dma_cfg
> when the cpu is configured in big endian mode.
>
> Signed-off-by: Arun Chandran
> ---
> This patch is tested on xilinx ZC702 evaluation board with
> CONFIG_CPU_BIG_ENDIAN=y and booting NFS rootfs
> Added on the fly IP endianness detection according to
> comments from Michal Simek.
> ---
> ---
> drivers/net/ethernet/cadence/macb.c | 33 +++--
> drivers/net/ethernet/cadence/macb.h | 15 ---
> 2 files changed, 35 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.c
> b/drivers/net/ethernet/cadence/macb.c
> index ad76b8e..1642911 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -449,7 +449,7 @@ static void macb_update_stats(struct macb *bp)
> WARN_ON((unsigned long)(end - p - 1) != (MACB_TPF - MACB_PFR) / 4);
>
> for(; p < end; p++, reg++)
> - *p += __raw_readl(reg);
> + *p += readl_relaxed(reg);
> }
>
> static int macb_halt_tx(struct macb *bp)
> @@ -1578,6 +1578,7 @@ static u32 macb_dbw(struct macb *bp)
> static void macb_configure_dma(struct macb *bp)
> {
> u32 dmacfg;
> + u32 tmp, ncr;
>
> if (macb_is_gem(bp)) {
> dmacfg = gem_readl(bp, DMACFG) & ~GEM_BF(RXBS, -1L);
> @@ -1585,7 +1586,25 @@ static void macb_configure_dma(struct macb *bp)
> if (bp->dma_burst_length)
> dmacfg = GEM_BFINS(FBLDO, bp->dma_burst_length, dmacfg);
> dmacfg |= GEM_BIT(TXPBMS) | GEM_BF(RXBMS, -1L);
> - dmacfg &= ~GEM_BIT(ENDIA);
> + dmacfg &= ~GEM_BIT(ENDIA_PKT);
> +
> + /* Here we use the loopback bit of net_ctrl register to detect
> + * endianness on IP. save it first. Program swaped mode for
> + * management descriptor accesses if writing to loop back bit
> + * and reading it back brings no change in bit value.
> + */
> + ncr = macb_readl(bp, NCR);
> + __raw_writel(MACB_BIT(LLB), bp->regs + MACB_NCR);
> + tmp = __raw_readl(bp->regs + MACB_NCR);
I have tested this patch on zc702 le and be and it is working.
The comment is not fully accurate. Sorry I thought that you are using a little
bit different
configuration than Zynq. The case I had in mind is that CPU and also IP can
have different endianness.
The code above is not checking endianness on IP itself but CPU endianness
because IP is in little
endian mode all the time. But the logic for detecting this on CPU is correct
and this is what
you need to do on Zynq. You are writing big endian value to little endian
register and read it back
to see if it was correctly written or not.
Can you please fix that comment to reflect this?
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
signature.asc
Description: OpenPGP digital signature
Re: [PATCH] net: macb: Add big endian CPU support
On Wed, Feb 25, 2015 at 3:32 PM, Michal Simek wrote:
> On 02/24/2015 08:39 AM, Arun Chandran wrote:
>> This patch converts all __raw_readl and __raw_writel function calls
>> to their corresponding readl_relaxed and writel_relaxed variants.
>>
>> It also tells the driver to set ahb_endian_swp_mgmt_en bit in dma_cfg
>> when the cpu is configured in big endian mode.
>>
>> Signed-off-by: Arun Chandran
>> ---
>> This patch is tested on xilinx ZC702 evaluation board with
>> CONFIG_CPU_BIG_ENDIAN=y and booting NFS rootfs
>> Added on the fly IP endianness detection according to
>> comments from Michal Simek.
>> ---
>> ---
>> drivers/net/ethernet/cadence/macb.c | 33 +++--
>> drivers/net/ethernet/cadence/macb.h | 15 ---
>> 2 files changed, 35 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/cadence/macb.c
>> b/drivers/net/ethernet/cadence/macb.c
>> index ad76b8e..1642911 100644
>> --- a/drivers/net/ethernet/cadence/macb.c
>> +++ b/drivers/net/ethernet/cadence/macb.c
>> @@ -449,7 +449,7 @@ static void macb_update_stats(struct macb *bp)
>> WARN_ON((unsigned long)(end - p - 1) != (MACB_TPF - MACB_PFR) / 4);
>>
>> for(; p < end; p++, reg++)
>> - *p += __raw_readl(reg);
>> + *p += readl_relaxed(reg);
>> }
>>
>> static int macb_halt_tx(struct macb *bp)
>> @@ -1578,6 +1578,7 @@ static u32 macb_dbw(struct macb *bp)
>> static void macb_configure_dma(struct macb *bp)
>> {
>> u32 dmacfg;
>> + u32 tmp, ncr;
>>
>> if (macb_is_gem(bp)) {
>> dmacfg = gem_readl(bp, DMACFG) & ~GEM_BF(RXBS, -1L);
>> @@ -1585,7 +1586,25 @@ static void macb_configure_dma(struct macb *bp)
>> if (bp->dma_burst_length)
>> dmacfg = GEM_BFINS(FBLDO, bp->dma_burst_length,
>> dmacfg);
>> dmacfg |= GEM_BIT(TXPBMS) | GEM_BF(RXBMS, -1L);
>> - dmacfg &= ~GEM_BIT(ENDIA);
>> + dmacfg &= ~GEM_BIT(ENDIA_PKT);
>> +
>> + /* Here we use the loopback bit of net_ctrl register to detect
>> + * endianness on IP. save it first. Program swaped mode for
>> + * management descriptor accesses if writing to loop back bit
>> + * and reading it back brings no change in bit value.
>> + */
>> + ncr = macb_readl(bp, NCR);
>> + __raw_writel(MACB_BIT(LLB), bp->regs + MACB_NCR);
>> + tmp = __raw_readl(bp->regs + MACB_NCR);
>
>
> I have tested this patch on zc702 le and be and it is working.
> The comment is not fully accurate. Sorry I thought that you are using a
> little bit different
> configuration than Zynq. The case I had in mind is that CPU and also IP can
> have different endianness.
>
> The code above is not checking endianness on IP itself but CPU endianness
> because IP is in little
Ok. I will change the comment.
> endian mode all the time. But the logic for detecting this on CPU is correct
> and this is what
> you need to do on Zynq. You are writing big endian value to little endian
> register and read it back
> to see if it was correctly written or not.
Just curious; will the same code work without change on a CPU with Big
endian IP?
(ie. If zynq hardware comes with only one change; IP configured in BE;
will the driver be needing further changes?)
--Arun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net: macb: Add big endian CPU support
>> endian mode all the time. But the logic for detecting this on CPU is correct >> and this is what >> you need to do on Zynq. You are writing big endian value to little endian >> register and read it back >> to see if it was correctly written or not. > > Just curious; will the same code work without change on a CPU with Big > endian IP? > (ie. If zynq hardware comes with only one change; IP configured in BE; > will the driver be needing further changes?) Depends on HW setup and architecture which you want to use. Let's say test could be to have Microblaze in PL in BE mode (not supported by tools) and bridge to axi. Then depends on Linux kernel how IO functions are implemented. That's why it is hard to say yes/no. Thanks, Michal -- Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/ Maintainer of Linux kernel - Xilinx Zynq ARM architecture Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform signature.asc Description: OpenPGP digital signature
Re: [PATCH] net: macb: Add big endian CPU support
Hi Arun, On 02/26/2015 11:44 AM, Arun Chandran wrote: > This patch converts all __raw_readl and __raw_writel function calls > to their corresponding readl_relaxed and writel_relaxed variants. > > It also tells the driver to set ahb_endian_swp_mgmt_en bit in dma_cfg > when the CPU is configured in big endian mode. > > Signed-off-by: Arun Chandran > --- > This patch is tested on xilinx ZC702 evaluation board with > CONFIG_CPU_BIG_ENDIAN=y and booting NFS rootfs > Added on the fly IP endianness detection according to > comments from Michal Simek. > Corrected GEM_* macros as per comments from Nicolas Ferre > --- > --- > drivers/net/ethernet/cadence/macb.c | 32 ++-- > drivers/net/ethernet/cadence/macb.h | 18 ++ > 2 files changed, 36 insertions(+), 14 deletions(-) > Can you please send regular v3 and list all changes you have done? Thanks, Michal -- Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/ Maintainer of Linux kernel - Xilinx Zynq ARM architecture Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform signature.asc Description: OpenPGP digital signature
Re: [PATCH] net: macb: Add big endian CPU support
Le 18/02/2015 12:29, Arun Chandran a écrit :
> This patch converts all __raw_readl and __raw_writel function calls
> to their corresponding readl_relaxed and writel_relaxed variants.
>
> It also tells the driver to set ahb_endian_swp_mgmt_en bit in dma_cfg
> when the CPU is configured in big endian mode.
>
> Signed-off-by: Arun Chandran
> ---
> This patch is tested on xilinx ZC702 evaluation board with
> CONFIG_CPU_BIG_ENDIAN=y and booting NFS rootfs
It seems okay:
Acked-by: Nicolas Ferre
> ---
> ---
> drivers/net/ethernet/cadence/macb.c | 18 --
> drivers/net/ethernet/cadence/macb.h | 15 ---
> 2 files changed, 20 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.c
> b/drivers/net/ethernet/cadence/macb.c
> index ad76b8e..05fb36d 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -449,7 +449,7 @@ static void macb_update_stats(struct macb *bp)
> WARN_ON((unsigned long)(end - p - 1) != (MACB_TPF - MACB_PFR) / 4);
>
> for(; p < end; p++, reg++)
> - *p += __raw_readl(reg);
> + *p += readl_relaxed(reg);
> }
>
> static int macb_halt_tx(struct macb *bp)
> @@ -1585,7 +1585,11 @@ static void macb_configure_dma(struct macb *bp)
> if (bp->dma_burst_length)
> dmacfg = GEM_BFINS(FBLDO, bp->dma_burst_length, dmacfg);
> dmacfg |= GEM_BIT(TXPBMS) | GEM_BF(RXBMS, -1L);
> - dmacfg &= ~GEM_BIT(ENDIA);
> + dmacfg &= ~GEM_BIT(ENDIA_PKT);
> + /* Tell the chip to byteswap descriptors on big-endian hosts */
> +#ifdef __BIG_ENDIAN
> + dmacfg |= GEM_BIT(ENDIA_DESC);
> +#endif
> if (bp->dev->features & NETIF_F_HW_CSUM)
> dmacfg |= GEM_BIT(TXCOEN);
> else
> @@ -1832,14 +1836,14 @@ static void gem_update_stats(struct macb *bp)
>
> for (i = 0; i < GEM_STATS_LEN; ++i, ++p) {
> u32 offset = gem_statistics[i].offset;
> - u64 val = __raw_readl(bp->regs + offset);
> + u64 val = readl_relaxed(bp->regs + offset);
>
> bp->ethtool_stats[i] += val;
> *p += val;
>
> if (offset == GEM_OCTTXL || offset == GEM_OCTRXL) {
> /* Add GEM_OCTTXH, GEM_OCTRXH */
> - val = __raw_readl(bp->regs + offset + 4);
> + val = readl_relaxed(bp->regs + offset + 4);
> bp->ethtool_stats[i] += ((u64)val) << 32;
> *(++p) += val;
> }
> @@ -2191,12 +2195,14 @@ static void macb_probe_queues(void __iomem *mem,
> *num_queues = 1;
>
> /* is it macb or gem ? */
> - mid = __raw_readl(mem + MACB_MID);
> + mid = readl_relaxed(mem + MACB_MID);
> +
> if (MACB_BFEXT(IDNUM, mid) != 0x2)
> return;
>
> /* bit 0 is never set but queue 0 always exists */
> - *queue_mask = __raw_readl(mem + GEM_DCFG6) & 0xff;
> + *queue_mask = readl_relaxed(mem + GEM_DCFG6) & 0xff;
> +
> *queue_mask |= 0x1;
>
> for (hw_q = 1; hw_q < MACB_MAX_QUEUES; ++hw_q)
> diff --git a/drivers/net/ethernet/cadence/macb.h
> b/drivers/net/ethernet/cadence/macb.h
> index 31dc080..57f0a1a 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -229,7 +229,8 @@
> /* Bitfields in DMACFG. */
> #define GEM_FBLDO_OFFSET 0 /* fixed burst length for DMA */
> #define GEM_FBLDO_SIZE 5
> -#define GEM_ENDIA_OFFSET 7 /* endian swap mode for packet data access */
> +#define GEM_ENDIA_DESC_OFFSET6 /* endian swap mode for management
> descriptor access */
> +#define GEM_ENDIA_PKT_OFFSET 7 /* endian swap mode for packet data access */
> #define GEM_ENDIA_SIZE 1
> #define GEM_RXBMS_OFFSET 8 /* RX packet buffer memory size select */
> #define GEM_RXBMS_SIZE 2
> @@ -423,17 +424,17 @@
>
> /* Register access macros */
> #define macb_readl(port,reg) \
> - __raw_readl((port)->regs + MACB_##reg)
> + readl_relaxed((port)->regs + MACB_##reg)
> #define macb_writel(port,reg,value) \
> - __raw_writel((value), (port)->regs + MACB_##reg)
> + writel_relaxed((value), (port)->regs + MACB_##reg)
> #define gem_readl(port, reg) \
> - __raw_readl((port)->regs + GEM_##reg)
> + readl_relaxed((port)->regs + GEM_##reg)
> #define gem_writel(port, reg, value) \
> - __raw_writel((value), (port)->regs + GEM_##reg)
> + writel_relaxed((value), (port)->regs + GEM_##reg)
> #define queue_readl(queue, reg) \
> - __raw_readl((queue)->bp->regs + (queue)->reg)
> + readl_relaxed((queue)->bp->regs + (queue)->reg)
> #define queue_writel(queue, reg, value) \
> - __raw_writel((value), (queue)->bp->r
Re: [PATCH] net: macb: Add big endian CPU support
On 02/18/2015 12:29 PM, Arun Chandran wrote: > This patch converts all __raw_readl and __raw_writel function calls > to their corresponding readl_relaxed and writel_relaxed variants. > > It also tells the driver to set ahb_endian_swp_mgmt_en bit in dma_cfg > when the CPU is configured in big endian mode. > > Signed-off-by: Arun Chandran > --- > This patch is tested on xilinx ZC702 evaluation board with > CONFIG_CPU_BIG_ENDIAN=y and booting NFS rootfs > --- > --- > drivers/net/ethernet/cadence/macb.c | 18 -- > drivers/net/ethernet/cadence/macb.h | 15 --- > 2 files changed, 20 insertions(+), 13 deletions(-) > > diff --git a/drivers/net/ethernet/cadence/macb.c > b/drivers/net/ethernet/cadence/macb.c > index ad76b8e..05fb36d 100644 > --- a/drivers/net/ethernet/cadence/macb.c > +++ b/drivers/net/ethernet/cadence/macb.c > @@ -449,7 +449,7 @@ static void macb_update_stats(struct macb *bp) > WARN_ON((unsigned long)(end - p - 1) != (MACB_TPF - MACB_PFR) / 4); > > for(; p < end; p++, reg++) > - *p += __raw_readl(reg); > + *p += readl_relaxed(reg); > } > > static int macb_halt_tx(struct macb *bp) > @@ -1585,7 +1585,11 @@ static void macb_configure_dma(struct macb *bp) > if (bp->dma_burst_length) > dmacfg = GEM_BFINS(FBLDO, bp->dma_burst_length, dmacfg); > dmacfg |= GEM_BIT(TXPBMS) | GEM_BF(RXBMS, -1L); > - dmacfg &= ~GEM_BIT(ENDIA); > + dmacfg &= ~GEM_BIT(ENDIA_PKT); > + /* Tell the chip to byteswap descriptors on big-endian hosts */ > +#ifdef __BIG_ENDIAN > + dmacfg |= GEM_BIT(ENDIA_DESC); > +#endif I don't think this is the best way what you should do. Instead of having this ifdef here you should find out any reg and detect if the IP is in big endian or little endian mode. I have done it for some xilinx IPs which can run on big or little endian system. In general find reg which some field which has some meaning - write there 1 and read expected value and based on that decide if you are on big or little endian system. Thanks, Michal -- Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/ Maintainer of Linux kernel - Xilinx Zynq ARM architecture Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform signature.asc Description: OpenPGP digital signature
Re: [PATCH] net: macb: Add big endian CPU support
On Thu, Feb 19, 2015 at 5:52 PM, Michal Simek wrote: > > On 02/18/2015 12:29 PM, Arun Chandran wrote: > > This patch converts all __raw_readl and __raw_writel function calls > > to their corresponding readl_relaxed and writel_relaxed variants. > > > > It also tells the driver to set ahb_endian_swp_mgmt_en bit in dma_cfg > > when the CPU is configured in big endian mode. > > > > Signed-off-by: Arun Chandran > > --- > > This patch is tested on xilinx ZC702 evaluation board with > > CONFIG_CPU_BIG_ENDIAN=y and booting NFS rootfs > > --- > > --- > > drivers/net/ethernet/cadence/macb.c | 18 -- > > drivers/net/ethernet/cadence/macb.h | 15 --- > > 2 files changed, 20 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/net/ethernet/cadence/macb.c > > b/drivers/net/ethernet/cadence/macb.c > > index ad76b8e..05fb36d 100644 > > --- a/drivers/net/ethernet/cadence/macb.c > > +++ b/drivers/net/ethernet/cadence/macb.c > > @@ -449,7 +449,7 @@ static void macb_update_stats(struct macb *bp) > > WARN_ON((unsigned long)(end - p - 1) != (MACB_TPF - MACB_PFR) / 4); > > > > for(; p < end; p++, reg++) > > - *p += __raw_readl(reg); > > + *p += readl_relaxed(reg); > > } > > > > static int macb_halt_tx(struct macb *bp) > > @@ -1585,7 +1585,11 @@ static void macb_configure_dma(struct macb *bp) > > if (bp->dma_burst_length) > > dmacfg = GEM_BFINS(FBLDO, bp->dma_burst_length, > > dmacfg); > > dmacfg |= GEM_BIT(TXPBMS) | GEM_BF(RXBMS, -1L); > > - dmacfg &= ~GEM_BIT(ENDIA); > > + dmacfg &= ~GEM_BIT(ENDIA_PKT); > > + /* Tell the chip to byteswap descriptors on big-endian hosts > > */ > > +#ifdef __BIG_ENDIAN > > + dmacfg |= GEM_BIT(ENDIA_DESC); > > +#endif > > I don't think this is the best way what you should do. > Instead of having this ifdef here you should find out any reg and detect if > the IP > is in big endian or little endian mode. I have done it for some xilinx IPs > which > can run on big or little endian system. > In general find reg which some field which has some meaning - write there 1 > and read expected value and based on that decide if you are on big or little > endian system. Hi Michal, I was not able to find any such registers for GEM in the TRM http://www.xilinx.com/support/documentation/user_guides/ug585-Zynq-7000-TRM.pdf I tried writing to dma_cfg (0x0010) and reading from design_cfg5 (0x0290) Its not reflecting anything(design_cfg5 always reads same value); It is not helping. The only way (I don't think its not right way either) I can think of eliminating that #ifdef __BIG_ENDIAN is reading some register in ARM cpu to identify its current endianness and write dma_cfg accordingly. --Arun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net: macb: Add big endian CPU support
Hi Arun,
On 02/20/2015 12:45 PM, Arun Chandran wrote:
> On Thu, Feb 19, 2015 at 5:52 PM, Michal Simek wrote:
>>
>> On 02/18/2015 12:29 PM, Arun Chandran wrote:
>>> This patch converts all __raw_readl and __raw_writel function calls
>>> to their corresponding readl_relaxed and writel_relaxed variants.
>>>
>>> It also tells the driver to set ahb_endian_swp_mgmt_en bit in dma_cfg
>>> when the CPU is configured in big endian mode.
>>>
>>> Signed-off-by: Arun Chandran
>>> ---
>>> This patch is tested on xilinx ZC702 evaluation board with
>>> CONFIG_CPU_BIG_ENDIAN=y and booting NFS rootfs
>>> ---
>>> ---
>>> drivers/net/ethernet/cadence/macb.c | 18 --
>>> drivers/net/ethernet/cadence/macb.h | 15 ---
>>> 2 files changed, 20 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/cadence/macb.c
>>> b/drivers/net/ethernet/cadence/macb.c
>>> index ad76b8e..05fb36d 100644
>>> --- a/drivers/net/ethernet/cadence/macb.c
>>> +++ b/drivers/net/ethernet/cadence/macb.c
>>> @@ -449,7 +449,7 @@ static void macb_update_stats(struct macb *bp)
>>> WARN_ON((unsigned long)(end - p - 1) != (MACB_TPF - MACB_PFR) / 4);
>>>
>>> for(; p < end; p++, reg++)
>>> - *p += __raw_readl(reg);
>>> + *p += readl_relaxed(reg);
>>> }
>>>
>>> static int macb_halt_tx(struct macb *bp)
>>> @@ -1585,7 +1585,11 @@ static void macb_configure_dma(struct macb *bp)
>>> if (bp->dma_burst_length)
>>> dmacfg = GEM_BFINS(FBLDO, bp->dma_burst_length,
>>> dmacfg);
>>> dmacfg |= GEM_BIT(TXPBMS) | GEM_BF(RXBMS, -1L);
>>> - dmacfg &= ~GEM_BIT(ENDIA);
>>> + dmacfg &= ~GEM_BIT(ENDIA_PKT);
>>> + /* Tell the chip to byteswap descriptors on big-endian hosts
>>> */
>>> +#ifdef __BIG_ENDIAN
>>> + dmacfg |= GEM_BIT(ENDIA_DESC);
>>> +#endif
>>
>> I don't think this is the best way what you should do.
>> Instead of having this ifdef here you should find out any reg and detect if
>> the IP
>> is in big endian or little endian mode. I have done it for some xilinx IPs
>> which
>> can run on big or little endian system.
>> In general find reg which some field which has some meaning - write there 1
>> and read expected value and based on that decide if you are on big or little
>> endian system.
>
> Hi Michal,
>
> I was not able to find any such registers for GEM in the TRM
> http://www.xilinx.com/support/documentation/user_guides/ug585-Zynq-7000-TRM.pdf
>
> I tried writing to dma_cfg (0x0010) and reading from design_cfg5
> (0x0290)
> Its not reflecting anything(design_cfg5 always reads same value); It
> is not helping.
>
> The only way (I don't think its not right way either) I can think of
> eliminating that
> #ifdef __BIG_ENDIAN is reading some register in ARM cpu to identify its
> current endianness and write dma_cfg accordingly.
Definitely no to detect cpu endianess.
What about this?
writel(0x2, 0xE000b000); //write little endian
if (readl(0xE000b000) == 0x2) { //read little endian
printf("little endian\n")
disable 0x2 bit (loopback)
} else {
printf("big endian\n");
definitely good to check writing 0x2 here that IP is
in big endian and reacts - if not BUG()
}
I have written this to spi-xilinx.c and you can use similar construction
for macb too (of course with checking above).
/*
* Detect endianess on the IP via loop bit in CR. Detection
* must be done before reset is sent because incorrect reset
* value generates error interrupt.
* Setup little endian helper functions first and try to use them
* and check if bit was correctly setup or not.
*/
xspi->read_fn = xspi_read32;
xspi->write_fn = xspi_write32;
xspi->write_fn(XSPI_CR_LOOP, xspi->regs + XSPI_CR_OFFSET);
tmp = xspi->read_fn(xspi->regs + XSPI_CR_OFFSET);
tmp &= XSPI_CR_LOOP;
if (tmp != XSPI_CR_LOOP) {
xspi->read_fn = xspi_read32_be;
xspi->write_fn = xspi_write32_be;
}
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
signature.asc
Description: OpenPGP digital signature
Re: [PATCH] net: macb: Add big endian CPU support
Le 20/02/2015 12:45, Arun Chandran a écrit : > On Thu, Feb 19, 2015 at 5:52 PM, Michal Simek wrote: >> >> On 02/18/2015 12:29 PM, Arun Chandran wrote: >>> This patch converts all __raw_readl and __raw_writel function calls >>> to their corresponding readl_relaxed and writel_relaxed variants. >>> >>> It also tells the driver to set ahb_endian_swp_mgmt_en bit in dma_cfg >>> when the CPU is configured in big endian mode. >>> >>> Signed-off-by: Arun Chandran >>> --- >>> This patch is tested on xilinx ZC702 evaluation board with >>> CONFIG_CPU_BIG_ENDIAN=y and booting NFS rootfs >>> --- >>> --- >>> drivers/net/ethernet/cadence/macb.c | 18 -- >>> drivers/net/ethernet/cadence/macb.h | 15 --- >>> 2 files changed, 20 insertions(+), 13 deletions(-) >>> >>> diff --git a/drivers/net/ethernet/cadence/macb.c >>> b/drivers/net/ethernet/cadence/macb.c >>> index ad76b8e..05fb36d 100644 >>> --- a/drivers/net/ethernet/cadence/macb.c >>> +++ b/drivers/net/ethernet/cadence/macb.c >>> @@ -449,7 +449,7 @@ static void macb_update_stats(struct macb *bp) >>> WARN_ON((unsigned long)(end - p - 1) != (MACB_TPF - MACB_PFR) / 4); >>> >>> for(; p < end; p++, reg++) >>> - *p += __raw_readl(reg); >>> + *p += readl_relaxed(reg); >>> } >>> >>> static int macb_halt_tx(struct macb *bp) >>> @@ -1585,7 +1585,11 @@ static void macb_configure_dma(struct macb *bp) >>> if (bp->dma_burst_length) >>> dmacfg = GEM_BFINS(FBLDO, bp->dma_burst_length, >>> dmacfg); >>> dmacfg |= GEM_BIT(TXPBMS) | GEM_BF(RXBMS, -1L); >>> - dmacfg &= ~GEM_BIT(ENDIA); >>> + dmacfg &= ~GEM_BIT(ENDIA_PKT); >>> + /* Tell the chip to byteswap descriptors on big-endian hosts >>> */ >>> +#ifdef __BIG_ENDIAN >>> + dmacfg |= GEM_BIT(ENDIA_DESC); >>> +#endif >> >> I don't think this is the best way what you should do. >> Instead of having this ifdef here you should find out any reg and detect if >> the IP >> is in big endian or little endian mode. I have done it for some xilinx IPs >> which >> can run on big or little endian system. >> In general find reg which some field which has some meaning - write there 1 >> and read expected value and based on that decide if you are on big or little >> endian system. > > Hi Michal, > > I was not able to find any such registers for GEM in the TRM > http://www.xilinx.com/support/documentation/user_guides/ug585-Zynq-7000-TRM.pdf > > I tried writing to dma_cfg (0x0010) and reading from design_cfg5 > (0x0290) > Its not reflecting anything(design_cfg5 always reads same value); It > is not helping. 0kay, here you mean that the design_cfg5 (@0x290) and its bit 16:15 "gem_endian_swap_def" doesn't change depending on the big/little endian configuration you use? I was also thinking about this configuration bit... too bad... Bye, > The only way (I don't think its not right way either) I can think of > eliminating that > #ifdef __BIG_ENDIAN is reading some register in ARM cpu to identify its > current endianness and write dma_cfg accordingly. > > --Arun > > -- Nicolas Ferre -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net: macb: Add big endian CPU support
On Mon, Feb 23, 2015 at 8:08 PM, Nicolas Ferre wrote: > Le 20/02/2015 12:45, Arun Chandran a écrit : >> On Thu, Feb 19, 2015 at 5:52 PM, Michal Simek wrote: >>> >>> On 02/18/2015 12:29 PM, Arun Chandran wrote: This patch converts all __raw_readl and __raw_writel function calls to their corresponding readl_relaxed and writel_relaxed variants. It also tells the driver to set ahb_endian_swp_mgmt_en bit in dma_cfg when the CPU is configured in big endian mode. Signed-off-by: Arun Chandran --- This patch is tested on xilinx ZC702 evaluation board with CONFIG_CPU_BIG_ENDIAN=y and booting NFS rootfs --- --- drivers/net/ethernet/cadence/macb.c | 18 -- drivers/net/ethernet/cadence/macb.h | 15 --- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c index ad76b8e..05fb36d 100644 --- a/drivers/net/ethernet/cadence/macb.c +++ b/drivers/net/ethernet/cadence/macb.c @@ -449,7 +449,7 @@ static void macb_update_stats(struct macb *bp) WARN_ON((unsigned long)(end - p - 1) != (MACB_TPF - MACB_PFR) / 4); for(; p < end; p++, reg++) - *p += __raw_readl(reg); + *p += readl_relaxed(reg); } static int macb_halt_tx(struct macb *bp) @@ -1585,7 +1585,11 @@ static void macb_configure_dma(struct macb *bp) if (bp->dma_burst_length) dmacfg = GEM_BFINS(FBLDO, bp->dma_burst_length, dmacfg); dmacfg |= GEM_BIT(TXPBMS) | GEM_BF(RXBMS, -1L); - dmacfg &= ~GEM_BIT(ENDIA); + dmacfg &= ~GEM_BIT(ENDIA_PKT); + /* Tell the chip to byteswap descriptors on big-endian hosts */ +#ifdef __BIG_ENDIAN + dmacfg |= GEM_BIT(ENDIA_DESC); +#endif >>> >>> I don't think this is the best way what you should do. >>> Instead of having this ifdef here you should find out any reg and detect if >>> the IP >>> is in big endian or little endian mode. I have done it for some xilinx IPs >>> which >>> can run on big or little endian system. >>> In general find reg which some field which has some meaning - write there 1 >>> and read expected value and based on that decide if you are on big or >>> little endian system. >> >> Hi Michal, >> >> I was not able to find any such registers for GEM in the TRM >> http://www.xilinx.com/support/documentation/user_guides/ug585-Zynq-7000-TRM.pdf >> >> I tried writing to dma_cfg (0x0010) and reading from design_cfg5 >> (0x0290) >> Its not reflecting anything(design_cfg5 always reads same value); It >> is not helping. > > 0kay, here you mean that the design_cfg5 (@0x290) and its bit 16:15 > "gem_endian_swap_def" doesn't change depending on the big/little endian > configuration you use? Yes. That register always reads the same. -- Arun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net: macb: Add big endian CPU support
Le 24/02/2015 08:39, Arun Chandran a écrit :
> This patch converts all __raw_readl and __raw_writel function calls
> to their corresponding readl_relaxed and writel_relaxed variants.
>
> It also tells the driver to set ahb_endian_swp_mgmt_en bit in dma_cfg
> when the cpu is configured in big endian mode.
>
> Signed-off-by: Arun Chandran
> ---
> This patch is tested on xilinx ZC702 evaluation board with
> CONFIG_CPU_BIG_ENDIAN=y and booting NFS rootfs
> Added on the fly IP endianness detection according to
> comments from Michal Simek.
> ---
> ---
> drivers/net/ethernet/cadence/macb.c | 33 +++--
> drivers/net/ethernet/cadence/macb.h | 15 ---
> 2 files changed, 35 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.c
> b/drivers/net/ethernet/cadence/macb.c
> index ad76b8e..1642911 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -449,7 +449,7 @@ static void macb_update_stats(struct macb *bp)
> WARN_ON((unsigned long)(end - p - 1) != (MACB_TPF - MACB_PFR) / 4);
>
> for(; p < end; p++, reg++)
> - *p += __raw_readl(reg);
> + *p += readl_relaxed(reg);
> }
>
> static int macb_halt_tx(struct macb *bp)
> @@ -1578,6 +1578,7 @@ static u32 macb_dbw(struct macb *bp)
> static void macb_configure_dma(struct macb *bp)
> {
> u32 dmacfg;
> + u32 tmp, ncr;
>
> if (macb_is_gem(bp)) {
> dmacfg = gem_readl(bp, DMACFG) & ~GEM_BF(RXBS, -1L);
> @@ -1585,7 +1586,25 @@ static void macb_configure_dma(struct macb *bp)
> if (bp->dma_burst_length)
> dmacfg = GEM_BFINS(FBLDO, bp->dma_burst_length, dmacfg);
> dmacfg |= GEM_BIT(TXPBMS) | GEM_BF(RXBMS, -1L);
> - dmacfg &= ~GEM_BIT(ENDIA);
> + dmacfg &= ~GEM_BIT(ENDIA_PKT);
I think this will fail... (see below)
> +
> + /* Here we use the loopback bit of net_ctrl register to detect
> + * endianness on IP. save it first. Program swaped mode for
> + * management descriptor accesses if writing to loop back bit
> + * and reading it back brings no change in bit value.
> + */
> + ncr = macb_readl(bp, NCR);
> + __raw_writel(MACB_BIT(LLB), bp->regs + MACB_NCR);
> + tmp = __raw_readl(bp->regs + MACB_NCR);
> +
> + if (tmp == MACB_BIT(LLB))
> + dmacfg &= ~GEM_BIT(ENDIA_DESC);
Ditto
> + else
> + dmacfg |= GEM_BIT(ENDIA_DESC);
Ditto
> +
> + /* Restore net_ctrl */
> + macb_writel(bp, NCR, ncr);
> +
> if (bp->dev->features & NETIF_F_HW_CSUM)
> dmacfg |= GEM_BIT(TXCOEN);
> else
> @@ -1832,14 +1851,14 @@ static void gem_update_stats(struct macb *bp)
>
> for (i = 0; i < GEM_STATS_LEN; ++i, ++p) {
> u32 offset = gem_statistics[i].offset;
> - u64 val = __raw_readl(bp->regs + offset);
> + u64 val = readl_relaxed(bp->regs + offset);
>
> bp->ethtool_stats[i] += val;
> *p += val;
>
> if (offset == GEM_OCTTXL || offset == GEM_OCTRXL) {
> /* Add GEM_OCTTXH, GEM_OCTRXH */
> - val = __raw_readl(bp->regs + offset + 4);
> + val = readl_relaxed(bp->regs + offset + 4);
> bp->ethtool_stats[i] += ((u64)val) << 32;
> *(++p) += val;
> }
> @@ -2191,12 +2210,14 @@ static void macb_probe_queues(void __iomem *mem,
> *num_queues = 1;
>
> /* is it macb or gem ? */
> - mid = __raw_readl(mem + MACB_MID);
> + mid = readl_relaxed(mem + MACB_MID);
> +
> if (MACB_BFEXT(IDNUM, mid) != 0x2)
> return;
>
> /* bit 0 is never set but queue 0 always exists */
> - *queue_mask = __raw_readl(mem + GEM_DCFG6) & 0xff;
> + *queue_mask = readl_relaxed(mem + GEM_DCFG6) & 0xff;
> +
> *queue_mask |= 0x1;
>
> for (hw_q = 1; hw_q < MACB_MAX_QUEUES; ++hw_q)
> diff --git a/drivers/net/ethernet/cadence/macb.h
> b/drivers/net/ethernet/cadence/macb.h
> index 31dc080..57f0a1a 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -229,7 +229,8 @@
> /* Bitfields in DMACFG. */
> #define GEM_FBLDO_OFFSET 0 /* fixed burst length for DMA */
> #define GEM_FBLDO_SIZE 5
> -#define GEM_ENDIA_OFFSET 7 /* endian swap mode for packet data access */
> +#define GEM_ENDIA_DESC_OFFSET6 /* endian swap mode for management
> descriptor access */
> +#define GEM_ENDIA_PKT_OFFSET 7 /* endian swap mode for packet data access */
here you would need to define as well:
#define GEM_ENDIA_DESC_SIZE 1
#define GEM_ENDIA_PKT_SIZE 1
Otherwise, I suspect that the GEM_BIT() macro won't work well.
>
Re: [PATCH] net: macb: Add big endian CPU support
On Tue, Feb 24, 2015 at 6:27 PM, Nicolas Ferre wrote:
> Le 24/02/2015 08:39, Arun Chandran a écrit :
>> This patch converts all __raw_readl and __raw_writel function calls
>> to their corresponding readl_relaxed and writel_relaxed variants.
>>
>> It also tells the driver to set ahb_endian_swp_mgmt_en bit in dma_cfg
>> when the cpu is configured in big endian mode.
>>
>> Signed-off-by: Arun Chandran
>> ---
>> This patch is tested on xilinx ZC702 evaluation board with
>> CONFIG_CPU_BIG_ENDIAN=y and booting NFS rootfs
>> Added on the fly IP endianness detection according to
>> comments from Michal Simek.
>> ---
>> ---
>> drivers/net/ethernet/cadence/macb.c | 33 +++--
>> drivers/net/ethernet/cadence/macb.h | 15 ---
>> 2 files changed, 35 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/cadence/macb.c
>> b/drivers/net/ethernet/cadence/macb.c
>> index ad76b8e..1642911 100644
>> --- a/drivers/net/ethernet/cadence/macb.c
>> +++ b/drivers/net/ethernet/cadence/macb.c
>> @@ -449,7 +449,7 @@ static void macb_update_stats(struct macb *bp)
>> WARN_ON((unsigned long)(end - p - 1) != (MACB_TPF - MACB_PFR) / 4);
>>
>> for(; p < end; p++, reg++)
>> - *p += __raw_readl(reg);
>> + *p += readl_relaxed(reg);
>> }
>>
>> static int macb_halt_tx(struct macb *bp)
>> @@ -1578,6 +1578,7 @@ static u32 macb_dbw(struct macb *bp)
>> static void macb_configure_dma(struct macb *bp)
>> {
>> u32 dmacfg;
>> + u32 tmp, ncr;
>>
>> if (macb_is_gem(bp)) {
>> dmacfg = gem_readl(bp, DMACFG) & ~GEM_BF(RXBS, -1L);
>> @@ -1585,7 +1586,25 @@ static void macb_configure_dma(struct macb *bp)
>> if (bp->dma_burst_length)
>> dmacfg = GEM_BFINS(FBLDO, bp->dma_burst_length,
>> dmacfg);
>> dmacfg |= GEM_BIT(TXPBMS) | GEM_BF(RXBMS, -1L);
>> - dmacfg &= ~GEM_BIT(ENDIA);
>> + dmacfg &= ~GEM_BIT(ENDIA_PKT);
>
> I think this will fail... (see below)
>
>> +
>> + /* Here we use the loopback bit of net_ctrl register to detect
>> + * endianness on IP. save it first. Program swaped mode for
>> + * management descriptor accesses if writing to loop back bit
>> + * and reading it back brings no change in bit value.
>> + */
>> + ncr = macb_readl(bp, NCR);
>> + __raw_writel(MACB_BIT(LLB), bp->regs + MACB_NCR);
>> + tmp = __raw_readl(bp->regs + MACB_NCR);
>> +
>> + if (tmp == MACB_BIT(LLB))
>> + dmacfg &= ~GEM_BIT(ENDIA_DESC);
>
> Ditto
>
>> + else
>> + dmacfg |= GEM_BIT(ENDIA_DESC);
>
> Ditto
Actually GEM_BIT(ENDIA_DESC) won't fail as I have defined
GEM_ENDIA_DESC_OFFSET. But I failed to notice others (GEM_BF*)
I will send another version. Thank you for spotting it.
--Arun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
