drivers/block/systemace: replaced in16/out16 with more common readw/writew macros
PATCH 1/1 drivers/block/systemace: replaced in16/out16 with more common readw/writew macros
Most architectures don't have symbols "in16"/"out16" defined. Only Microblaze/PowerPC/Spark architectures do have them defined.
At the same time there're much more common macros "readw"/"writew" for 16-bit data access defined in most of architectures (in linux kernel header "io.h").
So use of "readw"/"writew" makes it possible to build this driver for virtually any architecture.
--- drivers/block/systemace.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/block/systemace.c +2 -2
@@ -67,7 +67,7 @@ static void ace_writew(u16 val, unsigned off)#endif }else -out16 (base + off, val); +writew (val, base + off); }static u16 ace_readw (unsigned off)
@@ -80,7 +80,7 @@ static u16 ace_readw(unsigned off)#endif }else -return in16 (base + off); +return readw (base + off); } static unsigned long systemace_read(int dev, unsigned long start,
Signature · 1 line
1.7.10.4
PATCH 1/1 drivers/block/systemace: replaced in16/out16 with more common readw/writew macros
2013/1/2 Alexey Brodkin <[email protected]>:
Quoted 29 lines
Most architectures don't have symbols "in16"/"out16" defined. Only Microblaze/PowerPC/Spark architectures do have them defined.
At the same time there're much more common macros "readw"/"writew" for 16-bit data access defined in most of architectures (in linux kernel header "
io.h").So use of "readw"/"writew" makes it possible to build this driver for virtually any architecture.
Signed-off-by Alexey Brodkin <[email protected]>--- drivers/block/systemace.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/block/systemace.c +1
-1@@ -67,7 +67,7 @@ static void ace_writew(u16 val, unsigned off)#endif }else -out16 (base + off, val); +writew (val, base + off); }static u16 ace_readw (unsigned off)
drivers/block/systemace.c static u16 ace_readw(unsigned off) · line 84
Quoted 6 lines
drivers/block/systemace.c +1
-1@@ -80,7 +80,7 @@ static u16 ace_readw(unsigned off)#endif }else -return in16 (base + off); +return readw (base + off); }
Same as before in this part of the code.
But idea is OK.
M
Signature · 5 lines
Michal Simek, Ing. (M.Eng) 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
PATCH 1/1 drivers/block/systemace: replaced in16/out16 with more common readw/writew macros
In reply to Alexey BrodkinDear Alexey Brodkin,
Quoted 11 lines · In message <[email protected]> you…
Most architectures don't have symbols "in16"/"out16" defined. Only Microblaze/PowerPC/Spark architectures do have them defined.
At the same time there're much more common macros "readw"/"writew" for 16-bit data access defined in most of architectures (in linux kernel header "
io.h").So use of "readw"/"writew" makes it possible to build this driver for virtually any architecture.
Signed-off-by Alexey Brodkin <[email protected]>
Did you actually test this on - say - any PowerPC board?
To my understanding, in16() / out16() perform big endian accesses (i. e. direct load / store operations) on PowerPC, while readw / writew do byte-swapping (for accessling little-endian things like PCI busses).
I speculate that this patch will break all big-endian systems.
Best regards,
Wolfgang Denk
Signature · 6 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de "There are three principal ways to lose money: wine, women, and engi- neers. While the first two are more pleasant, the third is by far the more certain." - Baron Rothschild, ca. 1800
PATCH 1/1 drivers/block/systemace: replaced in16/out16 with more common readw/writew macros
Unfortunately I didn't test it on any BE hardware - I don't have anything handy at the moment.
I got it tested on LE platform - though it has nothing to do with real BE behavior.
But as far as I can see from kernel headers "writew" does this - regardless CPU endianess it writes 16-bit word to specified memory location in LE order.
http://lxr.free-electrons.com/source/arch/microblaze/include/asm/io.h#L96
====
static inline void writew(unsigned short v, volatile void __iomem *addr)
{
*(volatile unsigned short __force *)addr = cpu_to_le16(v);
}====
So depending on SystemACE internal endianess we may or may not use "writew". If SystemACE is always LE then "writew"/"readw" will do its work as expected. If SystemACE has the same endianess as CPU does then we may use "__raw_writew"/"__raw_readw" for data access.
But here http://www.xilinx.com/support/documentation/ip_documentation/xps_sysace.pdf
it is said:
====
The Xilinx System ACE Compact Flash chip is a true little-endian device.
====
So I expect "writew"/"readw" to work correctly assuming not intermediate bridge is put between CPU and System ACE controller.
Would be very nice if Michal may check it on his hardware and let us know about his findings.
2013/1/3 Wolfgang Denk <[email protected]>:
Quoted 35 lines
Dear Alexey Brodkin,
Quoted 11 lines · In message <[email protected]> you…
Most architectures don't have symbols "in16"/"out16" defined. Only Microblaze/PowerPC/Spark architectures do have them defined.
At the same time there're much more common macros "readw"/"writew" for 16-bit data access defined in most of architectures (in linux kernel header "
io.h").So use of "readw"/"writew" makes it possible to build this driver for virtually any architecture.
Signed-off-by Alexey Brodkin <[email protected]>Did you actually test this on - say - any PowerPC board?
To my understanding,
in16()/out16()perform big endian accesses (i. e. direct load / store operations) on PowerPC, while readw / writew do byte-swapping (for accessling little-endian things like PCI busses).I speculate that this patch will break all big-endian systems.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de "There are three principal ways to lose money: wine, women, and engi- neers. While the first two are more pleasant, the third is by far the
more certain." - Baron Rothschild, ca. 1800
PATCH 1/1 drivers/block/systemace: replaced in16/out16 with more common readw/writew macros
In reply to Alexey BrodkinDear Alexey Brodkin,
Quoted 9 lines · In message <[email protected]> you…
Most architectures don't have symbols "in16"/"out16" defined. Only Microblaze/PowerPC/Spark architectures do have them defined.
At the same time there're much more common macros "readw"/"writew" for 16-bit data access defined in most of architectures (in linux kernel header "
io.h").So use of "readw"/"writew" makes it possible to build this driver for virtually any architecture.
NAK. As expected, this breaks big endian systems like PowerPC.
Tested on AMCC "katmai" board (PPC440SPe):
before (current master):
U-Boot 2013.01-rc2-00174-ge56cdd7 (Jan 03 2013 - 12:41:58)
...
=> fatls ace 0 mnt-xsa
./
../ 1250875 kernel.img
19071970 ramdisk.img
2 file(s), 2 dir(s)
After your patch applied:
U-Boot 2013.01-rc2-00174-ge56cdd7-dirty (Jan 03 2013 - 13:01:27)
...
=> fatls ace 0 mnt-xsa
**** ACE CompactFlash not found.
** Bad device size - ace 0 **
So clear NAK.
Best regards,
Wolfgang Denk
Signature · 4 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Every little picofarad has a nanohenry all its own. - Don Vonada
A message that is not in the archive (sent off-list or not yet received): CAML3pwX=dfC4vfRfg-Rft5Xuem7q4-7VjtmySvitzWzY3z8JBg@mail.gmail.com
PATCH 1/1 drivers/block/systemace: replaced in16/out16 with more common readw/writew macros
Dear Alexey,
please don't top post / full quote, and please keep the ML on Cc: - thanks.
Quoted 2 lines · In message <CAML3pwX=dfC4vfRfg-Rft5Xuem7q4-7VjtmySvitzWzY3z8JBg@mail.gmail.com>…
And what if you use "__raw_writew"/"__raw_readw" instead?
I'd rather not try that, as the __raw_* functions are missing the memory barriers that are mandatory for such device access.
Best regards,
Wolfgang Denk
Signature · 5 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Hegel was right when he said that we learn from history that man can never learn anything from history. - George Bernard Shaw
PATCH 1/1 drivers/block/systemace: replaced in16/out16 with more common readw/writew macros
Hi Wolfgang,
but then do you think if there's any way to use some generic read/write routines?
The problem is we use ml509 board loaded with Synopsys DW ACR700 core and we need to access SystemACE CF-card controller (to boot Linux kernel mainly).
Or should I just add conditional branch for ARC architecture?
Regards,
Alexey
2013/1/3 Wolfgang Denk <[email protected]>:
Quoted 21 lines
Dear Alexey,
please don't top post / full quote, and please keep the ML on Cc: - thanks.
Quoted 2 lines · In message <CAML3pwX=dfC4vfRfg-Rft5Xuem7q4-7VjtmySvitzWzY3z8JBg@mail.gmail.com>…
And what if you use "__raw_writew"/"__raw_readw" instead?
I'd rather not try that, as the __raw_* functions are missing the memory barriers that are mandatory for such device access.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Hegel was right when he said that we learn from history that man can
never learn anything from history. - George Bernard Shaw
PATCH 1/1 drivers/block/systemace: replaced in16/out16 with more common readw/writew macros
Dear Alexey,
Quoted 3 lines · In message <CAML3pwXy7_FiVqJvgmm4-aHbspdY6BnMsZonYfzcw_hA9Fe_fw@mail.gmail.com>…
but then do you think if there's any way to use some generic read/write routines?
For reference, please see section "KERNEL I/O BARRIER EFFECTS" in the Linux kernel Documentation/memory-barriers.txt file. According to that, we should strive to using ioreadX(), iowriteX() (and some kernel hackers actually support such an oppinion) - however, there is little support for thjis in existing code.
ARM traditionally tends to be using readX(), writeX(), at least on the LE implmentamtions (I don't know what the BE variants do). PowerPC has tradtitionally been using inX(), outX() resp. their explicit counterparts in_leX() / in_beX() / out_leX() / out_beX(). These functions are also available in ARM, and have been used for drivers for IP blocks shared between SoCs of different architectures.
Quoted 5 lines
The problem is we use ml509 board loaded with Synopsys DW ACR700 core and we need to access SystemACE CF-card controller (to boot Linux kernel mainly).
Or should I just add conditional branch for ARC architecture?
Is there a need to change the driver at all? Can you not just provide the needed inX(), outX() accessors for your architecture?
Best regards,
Wolfgang Denk
Signature · 5 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de "Why waste negative entropy on comments, when you could use the same entropy to create bugs instead?" - Steve Elias
PATCH 1/1 drivers/block/systemace: replaced in16/out16 with more common readw/writew macros
In reply to Wolfgang Denk2013/1/3 Wolfgang Denk <[email protected]>:
Quoted 30 lines
Dear Alexey Brodkin,
Quoted 9 lines · In message <[email protected]> you…
Most architectures don't have symbols "in16"/"out16" defined. Only Microblaze/PowerPC/Spark architectures do have them defined.
At the same time there're much more common macros "readw"/"writew" for 16-bit data access defined in most of architectures (in linux kernel header "
io.h").So use of "readw"/"writew" makes it possible to build this driver for virtually any architecture.
NAK. As expected, this breaks big endian systems like PowerPC.
Tested on AMCC "katmai" board (PPC440SPe):
before (current master):
U-Boot 2013.01-rc2-00174-ge56cdd7 (Jan 03 2013 - 12:41:58)
...
=> fatls ace 0 mnt-xsa./ ../1250875 kernel.img
19071970 ramdisk.img2 file(s), 2 dir(s)
Quoted 8 lines
After your patch applied:
U-Boot 2013.01-rc2-00174-ge56cdd7-dirty (Jan 03 2013 - 13:01:27)
...
=> fatls ace 0 mnt-xsa
**** ACE CompactFlash not found.
** Bad device size - ace 0 **
Just a question is this 8bit width or 16bit?
I have checked with our hw guy and we don't have hw board for 8bit width.
Thanks,
Michal
Signature · 5 lines
Michal Simek, Ing. (M.Eng) 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
PATCH 1/1 drivers/block/systemace: replaced in16/out16 with more common readw/writew macros
Dear Michal Simek,
Quoted 11 lines · In message <CAHTX3dLp6AnYpbSz=jFjCZCbZMtAHb9_h-4hcv=VxcZHPnp8bQ@mail.gmail.com>…
Quoted 8 lines
After your patch applied:
U-Boot 2013.01-rc2-00174-ge56cdd7-dirty (Jan 03 2013 - 13:01:27)
...
=> fatls ace 0 mnt-xsa
**** ACE CompactFlash not found.
** Bad device size - ace 0 **Just a question is this 8bit width or 16bit?
I have checked with our hw guy and we don't have hw board for 8bit width.
-> grep CONFIG_SYS_SYSTEMACE_WIDTH include/configs/katmai.h
#define CONFIG_SYS_SYSTEMACE_WIDTH 16 /* Data bus width is 16 */
Best regards,
Wolfgang Denk
Signature · 5 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Living on Earth may be expensive, but it includes an annual free trip around the Sun.
PATCH 1/1 drivers/block/systemace: replaced in16/out16 with more common readw/writew macros
2013/1/3 Wolfgang Denk <[email protected]>:
Quoted 12 lines
Dear Michal Simek,
Quoted 9 lines · In message <CAHTX3dLp6AnYpbSz=jFjCZCbZMtAHb9_h-4hcv=VxcZHPnp8bQ@mail.gmail.com>…
Quoted 8 lines
After your patch applied:
U-Boot 2013.01-rc2-00174-ge56cdd7-dirty (Jan 03 2013 - 13:01:27)
...
=> fatls ace 0 mnt-xsa
**** ACE CompactFlash not found.
** Bad device size - ace 0 **
Quoted 5 lines
Quoted 2 lines
Just a question is this 8bit width or 16bit?
I have checked with our hw guy and we don't have hw board for 8bit width.-> grep
CONFIG_SYS_SYSTEMACE_WIDTHinclude/configs/katmai.h
#defineCONFIG_SYS_SYSTEMACE_WIDTH16 /* Data bus width is 16 */
Fair enough. I see that JSE is using 8 bit width - do you have this board for test?
Thanks,
Michal
Signature · 5 lines
Michal Simek, Ing. (M.Eng) 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
PATCH 1/1 drivers/block/systemace: replaced in16/out16 with more common readw/writew macros
Dear Michal Simek,
Quoted 6 lines · In message <CAHTX3d+=yQ2NdTDViEoK3rG7cSqpFuOd8RwgnExpJjANWTd1QQ@mail.gmail.com>…
Quoted 3 lines
-> grep
CONFIG_SYS_SYSTEMACE_WIDTHinclude/configs/katmai.h
#defineCONFIG_SYS_SYSTEMACE_WIDTH16 /* Data bus width is 16 */Fair enough. I see that JSE is using 8 bit width - do you have this board for test?
Sorry, we don't have any such board. I think the Katmai is the only one we have with a SystemACE on it.
Best regards,
Wolfgang Denk
Signature · 4 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de What is research but a blind date with knowledge? -- Will Harvey