RE: [linux-devel] [PATCH 1/2] spi: spi-fsl-dspi: replace regmap R/W with internal implementation

[Yao Yuan <yao.yuan@xxxxxxx>]

On Thu, May 12, 2016 at 09:41 OM, Po Liu wrote:
> On Thu, May12, 2016 at 06:20 AM, Leo Li Wrote:
> >  On Mon, May 9, 2016 at 5:58 AM, Yao Yuan <yao.yuan@xxxxxxx> wrote:
> >  > On Mon, May 09, 2016 at 06:29:59AM +0800, Mark Brown wrote:
> >  >> On Mon, May 09, 2016 at 08:48:59AM +0000, Yao Yuan wrote:
> >  >> > Hi Mark Brown,
> >  >>
> >  >> Please don't top post, reply in line with needed context.  This
> > >> allows readers to readily follow the flow of conversation and  >>
> > understand what you are talking about and also helps ensure that  >>
> > everything in the discussion is being addressed.
> >  >>
> >  >> Please fix your mail client to word wrap within paragraphs at  >>
> > something substantially less than 80 columns.  Doing this makes your
> > >> messages much easier to read and reply to.
> >  >>
> >  >
> >  > Ok, Thanks.
> >  >
> >  >> > There are two problems we have found for use regmap up to now.
> >  >>
> >  >> > 1, Regmap will read device value depends on endian in .dtsi(DSPI
> > in  >> > .dtsi file is  >> big-endian) and readl()(readl() in arm64 is
> > le32_to_cpu read) in  system.
> >  >> > That is the value read out is fixed regardless the kernel were
> > >> > compiled with  >> CONFIG_CPU_BIG_ENDIAN yes or no. That makes the
> > value(read from  >> DSPI)can't be recognized in one endian
> > mode(CONFIG_CPU_BIG_ENDIAN be  >> set or not).
> >  >> > The result is: DSPI can't working with CONFIG_CPU_BIG_ENDIAN set.
> >  >>
> >  >> This doesn't make sense.  If the endianness of the device is  >>
> > specified in DT then the endianness of the CPU won't make a
> > difference.
> >  >>
> >  >
> >  > For example, if DSPI controller register is BE, so I set big-endian
> > in  DT.
> >  > That means we should R/W the DSPI controller register with big-endian.
> >  > Then I can think all of the value we R/W to/from DSPI controller  >
> > register, we can Think they are the BE.
> >  > So that we can understand it easy.
> >  >
> >  > That means no matter core is LE or BE, we both need get the value
> > from  > DSPI register with big endian.
> >  >
> >  > But from regmap is not.
> >  > When the core is little endian, I can get the big endian value form
> > register like:0xaabbccdd.
> >  > Then we can analysis and process this value.
> >  > But once config the core to big endian, I get the value
> > like:0xddccbbaa. It's the little endian.
> >  > That's terrible for my driver to understand the value.
> >
> >  This is just the result.  Are you able to dig deeper to explain why
> > this  is happening?
> spi/spi-fsl-dspi.c use the devm_regmap_init_mmio_clk() get regmap which will
> use regmap_mmio for operation the registers.
> 
> static struct regmap_bus regmap_mmio = {
> 	...
> 	.write = regmap_mmio_write,
> 	.read = regmap_mmio_read,
> }
> 
> Anyway, the whole process register read operation will include
> regmap_mmio_read and format.parse_val(map->work_buf) then get the
> value.
> 	regmap_mmio_read() will use le32_to_cpu() value,
> 	format.parse_val() will use cpu_to_be32() (if .dtsi file config the dspi
> with big_endian, else use cpu_to_le32() if set little_endian )
> 
> So for kernel we get the value will always le32_to_cpu() + cpu_to_be32(), in
> short, the value always get value from register le32_to_be32 read. This
> operation could not be recognized for both big_endian and little_endian core
> setting. Founded that it will hang in one type of CPU ENDIAN mode.

Yes, That's the reason, Thanks for you to explain.

> 
> >
> >  I checked the LS1043 reference manual, the register definition looks
> > like little endian instead of the big endian defined in the LS1043
> > device tree.  Can you confirm if the block is actually having big
> > endian  registers or little endian registers.

No, it should be the document issue.
The register for DSPI on LS1043A is still big-endian.
Maybe will changed to little-endian in some later new SOC.

> >
> >  Regards,
> >  Leo
> >  _______________________________________________
> >  linux-devel mailing list
> >  linux-devel@xxxxxxxxxxxxxxxxxxxx
> >  http://gforge.freescale.net/mailman/listinfo/linux-devel
��.n��������+%������w��{.n�����{����)��jg��������ݢj����G�������j:+v���w�m������w�������h�����٥