Re: [PATCH] microblaze: intc: set the default irq_domain
2013/3/17 Dan Christensen : > Register the irq_domain created during initialization as the default so > that device drivers can pass NULL to irq_create_mapping and get a > virtual irq to pass to request_irq. > > Signed-off-by: Dan Christensen > --- > arch/microblaze/kernel/intc.c |2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/arch/microblaze/kernel/intc.c b/arch/microblaze/kernel/intc.c > index 8778adf..d85fa3a 100644 > --- a/arch/microblaze/kernel/intc.c > +++ b/arch/microblaze/kernel/intc.c > @@ -172,4 +172,6 @@ void __init init_IRQ(void) > * and commits this patch. ~~gcl */ > root_domain = irq_domain_add_linear(intc, nr_irq, > &xintc_irq_domain_ops, > (void *)intr_mask); > + > + irq_set_default_host(root_domain); > } > -- > 1.7.0.4 > Applied. Thanks, Michal -- 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 and responsible for u-boot arm zynq platform -- 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] memblock: Kill ARCH_POPULATES_NODE_MAP once more
On 03/20/2013 11:25 AM, Paul Bolle wrote: The Kconfig symbol ARCH_POPULATES_NODE_MAP was killed in v3.3. After that it popped up again in microblaze and metag. Nobody noticed, probably because these Kconfig symbols are entirely unused and these architectures both select HAVE_MEMBLOCK_NODE_MAP. Anyhow, these two entries can also be killed. Ah. We have done the developing on that on earlier version and to mainline it went in 3.3 merge window that's why I didn't notice it. If James wants to take this patch through his tree, I am ok with that and here is my Acked-by: Michal Simek Or if you want to take this through my microblaze tree, I am also ok with that but please send me your ACK. Thanks for this patch, Michal -- Michal Simek, Ing. (M.Eng) w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/ Microblaze U-BOOT custodian -- 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] memblock: Kill ARCH_POPULATES_NODE_MAP once more
2013/3/20 James Hogan : > On 20/03/13 11:02, Michal Simek wrote: >> On 03/20/2013 11:25 AM, Paul Bolle wrote: >>> The Kconfig symbol ARCH_POPULATES_NODE_MAP was killed in v3.3. After >>> that it popped up again in microblaze and metag. Nobody noticed, >>> probably because these Kconfig symbols are entirely unused and these >>> architectures both select HAVE_MEMBLOCK_NODE_MAP. Anyhow, these two >>> entries can also be killed. >> >> Ah. We have done the developing on that on earlier version and >> to mainline it went in 3.3 merge window that's why I didn't notice it. >> >> If James wants to take this patch through his tree, I am ok with that >> and here is my >> >> Acked-by: Michal Simek >> >> Or if you want to take this through my microblaze tree, I am also ok >> with that but please send me your ACK. > > Thanks for the patch Paul. This was added to metag when we were on > 2.6.34. I spotted the adding of HAVE_MEMBLOCK_NODE_MAP to SH in v3.3 and > did the same for metag, but didn't notice the removal of > ARCH_POPULATES_NODE_MAP. > > I'm happy to take this patch for v3.10. Ok. Thanks, Michal -- 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 and responsible for u-boot arm zynq platform -- 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] drivers/block/xsysace - replace in(out)_8/in(out)_be16/in(out)_le16 with generic iowrite(read)8/16(be)
HI, [cc: Alan and Geert and David] 2013/1/30 Arnd Bergmann : > On Wednesday 30 January 2013 13:31:58 Michal Simek wrote: >> Also from my understanding of arm we should use readl/b/w functions because >> they have memory barriers which should be probably performed. >> >> And I haven't found any IO function which will behave on arm as LE and >> on PPC as BE. > > There are none, except for the __raw_ versions that are generally > not recommended for other reasons. > > There really is no easy solution, because most of the hardware IP > blocks have fixed endianess, e.g. an OHCI USB controller will normally > have little-endian registers on all architectures, but in very rare > cases it may be big-endian, because some hardware designer tried to > be helpful and put a byte swapping logic in front of it. > > There are a few devices that tend to have the same endianess as the > CPU, again because some hardware designer tried to make things easy > for software by making the hardware confiurable. Again, what normally > happens is that the next hardware designer takes this block and > hardwires it to some endianess that he sees as "correct" but puts > it on a system with a variable-endian CPU. Note that there are both > little-endian PowerPC systems and big-endian ARM systems, they just > happen to be the minority on an architecture where 99% of the users > prefer the opposite. > > The outcome is basically you're screwed for every driver that is > not fixed endianess. The normal workaround is to do something like > > #if defined(CONFIG_FOO_BIG_ENDIAN) && !defined(CONFIG_FOO_LITTLE_ENDIAN) > /* always big-endian > #define foo_readl(dev, reg) ioread32_be(dev->regs + reg) > #define foo_writel(dev, reg, val) iowrite32_be(val, dev->regs + reg) > #elif !defined(CONFIG_FOO_BIG_ENDIAN) && defined(CONFIG_FOO_LITTLE_ENDIAN) > /* always little-endian > #define foo_readl(dev, reg) ioread32(dev->regs + reg) > #define foo_writel(dev, reg, val) iowrite32e(val, dev->regs + reg) > #else > /* run-time configured */ > #define foo_readl(dev, reg) dev->readl(dev->regs + reg) > #define foo_writel(dev, reg, val) dev->writel(val, dev->regs + reg) > #endif > > and select the CONFIG_FOO_BIG_ENDIAN and CONFIG_FOO_LITTLE_ENDIAN > symbols in Kconfig based on the system you are building for. Using CONFIG_FOO_BIG/LITTLE is not good because it is just another Kconfig option. You can easily detect it at runtime and for dedicated hw with fixed endians you can just handle it in the driver and don't care about global setting. > This of course gets further complicated if you require different > accessors per architecture, like ARM wanting readl or ioread32 > and PowerPC wanting in_le32 for a little-endian SoC component. FYI: I have got two responses on linux-arch from Alan "Set the pointers up and pass them as data with your platform device, that way the function definitions are buried in your platform code where they depend." and Geert: "Or embed a struct io_ops * in struct device, to be set up by the bus driver? Wasn't David Hinds working on something like this in the context of PCMCIA a few decades ago?" Based on their suggestions one way can be to pass it through void *platform_data which is probably not the best and then which make more sense to me is to extend struct dev_archdata archdata to add there native read/write functions. What do you think? Thanks, Michal -- 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 and responsible for u-boot arm zynq platform -- 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] drivers/block/xsysace - replace in(out)_8/in(out)_be16/in(out)_le16 with generic iowrite(read)8/16(be)
2013/2/4 Arnd Bergmann : > On Monday 04 February 2013, Michal Simek wrote: >> > >> > and select the CONFIG_FOO_BIG_ENDIAN and CONFIG_FOO_LITTLE_ENDIAN >> > symbols in Kconfig based on the system you are building for. >> >> Using CONFIG_FOO_BIG/LITTLE is not good because it is just another >> Kconfig option. >> You can easily detect it at runtime and for dedicated hw with fixed >> endians you can just >> handle it in the driver and don't care about global setting. > > The configuration option should not be visible, so it's > not something a user would have to worry about. It's just > sometimes nicer to express the configuration of the platform > in terms of Kconfig syntax than it is in C code if you have > complex platform dependencies. > >> > This of course gets further complicated if you require different >> > accessors per architecture, like ARM wanting readl or ioread32 >> > and PowerPC wanting in_le32 for a little-endian SoC component. >> >> FYI: I have got two responses on linux-arch from Alan >> "Set the pointers up and pass them as data with your platform device, that >> way the function definitions are buried in your platform code where they >> depend." >> >> and Geert: >> "Or embed a struct io_ops * in struct device, to be set up by the bus driver? >> >> Wasn't David Hinds working on something like this in the context of PCMCIA >> a few decades ago?" >> >> Based on their suggestions one way can be to pass it through void >> *platform_data >> which is probably not the best and then which make more sense to me is to >> extend >> struct dev_archdata archdata to add there native read/write functions. >> >> What do you think? > > I worry a little about code size if we have a lot of drivers that go > from one instruction to an indirect function call for each readl/writel. > Using platform_data ss also something that does not work too well with > device tree based platform configuration, which tries hard to leave > all run-time configuration inside of the driver. As you have seen from the list of architectures all of them uses device tree. Thanks, Michal -- 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 and responsible for u-boot arm zynq platform -- 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] drivers/block/xsysace - replace in(out)_8/in(out)_be16/in(out)_le16 with generic iowrite(read)8/16(be)
2013/2/5 Benjamin Herrenschmidt : > On Mon, 2013-02-04 at 17:24 +, Arnd Bergmann wrote: >> On Monday 04 February 2013, Michal Simek wrote: >> > > >> > > and select the CONFIG_FOO_BIG_ENDIAN and CONFIG_FOO_LITTLE_ENDIAN >> > > symbols in Kconfig based on the system you are building for. >> > >> > Using CONFIG_FOO_BIG/LITTLE is not good because it is just another >> > Kconfig option. >> > You can easily detect it at runtime and for dedicated hw with fixed >> > endians you can just >> > handle it in the driver and don't care about global setting. >> >> The configuration option should not be visible, so it's >> not something a user would have to worry about. It's just >> sometimes nicer to express the configuration of the platform >> in terms of Kconfig syntax than it is in C code if you have >> complex platform dependencies. > > As long as you never have a case where both are needed at runtime... > >> > > This of course gets further complicated if you require different >> > > accessors per architecture, like ARM wanting readl or ioread32 >> > > and PowerPC wanting in_le32 for a little-endian SoC component. > > powerpc should never "want" in_le32. ioread32 should work just as well. > in_le32 is historical stuff and is never required. > >> > FYI: I have got two responses on linux-arch from Alan >> > "Set the pointers up and pass them as data with your platform device, that >> > way the function definitions are buried in your platform code where they >> > depend." >> > >> > and Geert: >> > "Or embed a struct io_ops * in struct device, to be set up by the bus >> > driver? >> > >> > Wasn't David Hinds working on something like this in the context of PCMCIA >> > a few decades ago?" >> > >> > Based on their suggestions one way can be to pass it through void >> > *platform_data >> > which is probably not the best and then which make more sense to me is to >> > extend >> > struct dev_archdata archdata to add there native read/write functions. >> > >> > What do you think? >> >> I worry a little about code size if we have a lot of drivers that go >> from one instruction to an indirect function call for each readl/writel. >> Using platform_data ss also something that does not work too well with >> device tree based platform configuration, which tries hard to leave >> all run-time configuration inside of the driver. > > readl/writel and ioread32/iowrite32 should always be equivalent. > > So it all boils down to whether your device has different endianness (it > should not, doing so is stupid ... but if it really does then make it a > runtime wrapper that chooses between ioread32 and ioread32be xilinx ppc is big endian xilinx arm is little endian xilinx microblaze is little endian and big endian It is just sharing the same IP across all platforms. Which is better than create new devices and new device drivers for it. It means that all of them are register compatible but require access with native platform endianness as I listed above. It is not a problem to create runtime wrapper and even detect endian directly in the driver but the point if this is the proper design. Also ioread32 and ioread32be shouldn't be used on ARM because there are missing memory barriers. Thanks, Michal -- 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 and responsible for u-boot arm zynq platform -- 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] drivers/block/xsysace - replace in(out)_8/in(out)_be16/in(out)_le16 with generic iowrite(read)8/16(be)
2013/2/5 Benjamin Herrenschmidt : > On Tue, 2013-02-05 at 11:54 +0100, Michal Simek wrote: >> >> xilinx ppc is big endian >> xilinx arm is little endian >> xilinx microblaze is little endian and big endian > > You are talking about the core, why should the xsysace block driver be > synthesized with the same endianness as the core ? That's not terribly > useful, especially since reverse load/stores on ppc are essentially > free... It is done by hardware guys and I can do nothing with it. >> It is just sharing the same IP across all platforms. Which is better >> than create new devices and new device drivers for it. It means that >> all of them are register compatible but require access with native >> platform endianness as I listed above. > > Every attempt at doing "native platform endianness" has always been a > misguided attempt turning into a trainwreck (see OHCI USB). > > Just pick one endian for the device and stick to it. It is reality and I can't change it. Arnd mentioned it earlier that USB >> It is not a problem to create runtime wrapper and even detect endian >> directly in the driver >> but the point if this is the proper design. >> Also ioread32 and ioread32be shouldn't be used on ARM because there >> are missing memory barriers. > > Then fix them, they shouldn't be, it's a bug, it will break many other > drivers. They should be fully equivalent to readl. I want to be sure about this. I have parsed this again with closer look and seems to me that ioread32 is equal to readl and iowrite32 to writel. Arnd: Am I right? Thanks, Michal -- 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 and responsible for u-boot arm zynq platform -- 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] drivers/block/xsysace - replace in(out)_8/in(out)_be16/in(out)_le16 with generic iowrite(read)8/16(be)
2013/2/6 Grant Likely : > On Tue, Feb 5, 2013 at 3:12 PM, Arnd Bergmann wrote: >> On Tuesday 05 February 2013 18:03:31 Alexey Brodkin wrote: >>> The Xilinx System ACE Compact Flash chip is a true little-endian device >>> and the PLB is a big-endian bus. Therefore the XPS System ACE Interface >>> Controller will do a bit-swap in each byte when connecting the PLB data >>> bus to the System ACE data bus as shown in Table 2. >>> >>> Note however, that the XPS System ACE Interface Controller does not >>> perform the byte swapping necessary to interface to a little-endian >>> device when configured to use 16-bit mode. Therefore, the software >>> drivers provided for this core will perform the necessary byte-swapping >>> to correctly interface to the Xilinx System ACE Compact Flash chip as >>> shown in Table 3. >> >> Ok. In this case, I would recommend making the default for this driver >> little-endian, and adding a quirk for broken hardware bridges like the >> one you cited to have a mixed-endian mode if configured so at compile >> time. >> >> It seems that on all normal platforms, this device should behave as >> little-endian, while the Xilinx bridge can be either big-endian >> or little-endian, depending on whether it is used in 8-bit or 16-bit >> mode, so if we are using this, it cannot be known at compile time. > > The driver already handles this. It has three sets of accessors; > 8-bit, 16-bit LE and 16-bit BE *and* when doing 16-bit it figures out > on its own which set to use at runtime. There is nothing controversial > here. The only problem is that the driver is currently using in_/out_ > IO accessors instead of ioread/iowrite variants. I have looked at the patches from more practical side and I have tested it on microblaze big endian in 16bit mode and I have found that sysace driver stop to work. After that I have looked at ioread/iowrite microblaze implementation and implementation of that functions is wrong. I have fixed it but looking at using asm-generic/io.h for microblaze. I will do more tests and let you know. Thanks, Michal -- 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 and responsible for u-boot arm zynq platform -- 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] drivers/block/xsysace - replace in(out)_8/in(out)_be16/in(out)_le16 with generic iowrite(read)8/16(be)
On 02/07/2013 01:34 AM, Arnd Bergmann wrote: On Wednesday 06 February 2013 17:21:37 Michal Simek wrote: I have looked at the patches from more practical side and I have tested it on microblaze big endian in 16bit mode and I have found that sysace driver stop to work. After that I have looked at ioread/iowrite microblaze implementation and implementation of that functions is wrong. I have fixed it but looking at using asm-generic/io.h for microblaze. I will do more tests and let you know. Well, I think they are only wrong in the way that they ignore endianess. You can fix that by changing them to be identical to the in_le/in_be families. But still it is wrong. However, I would also recommend changing your __raw_* accessors to inline assembly functions rather than pointer dereferences, because we have had problems in the past where gcc (when faced with undefined C) silently turned 32-bit accesses into multiples of byte accesses, which can be fatal for MMIO. The asm-generic version obviously cannot get this right. The PCI I/O space handling, as mentioned, is completely broken on microblaze, and you can either use the approach from asm-generic when you set PCI_IOBASE match your isa_io_base. One question regarding to asm-generic/io.h about iowrite16be implementation #define iowrite16be(v, addr) iowrite16(be16_to_cpu(v), (addr)) #define iowrite16(v, addr) writew((v), (addr)) #define writew(b,addr) __raw_writew(__cpu_to_le16(b),addr) static inline void __raw_writew(u16 b, volatile void __iomem *addr) { *(volatile u16 __force *) addr = b; } How is this suppose to work on Big Endian? be16_to_cpu(v) is (v) and __cpu_to_le16(b) is swab16(v) On little be16_to_cpu(v) is swab16(v) and __cpu_to_le16(swab(b)) is swab16(v) What I would expect is #define iowrite16be(v, addr) __raw_writew(__cpu_to_be16(v), addr) on Big endian: __cpu_to_be16(v) is (v) on Little endian: __cpu_to_be16(v) is swab(v) What am I missing here? Thanks, Michal -- Michal Simek, Ing. (M.Eng) w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/ Microblaze U-BOOT custodian -- 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] drivers/block/xsysace - replace in(out)_8/in(out)_be16/in(out)_le16 with generic iowrite(read)8/16(be)
2013/2/6 Geert Uytterhoeven : > On Wed, Feb 6, 2013 at 6:40 PM, Michal Simek wrote: >> One question regarding to asm-generic/io.h about iowrite16be implementation >> >> #define iowrite16be(v, addr) iowrite16(be16_to_cpu(v), (addr)) >> #define iowrite16(v, addr) writew((v), (addr)) >> #define writew(b,addr) __raw_writew(__cpu_to_le16(b),addr) >> >> static inline void __raw_writew(u16 b, volatile void __iomem *addr) >> { >> *(volatile u16 __force *) addr = b; >> } >> >> How is this suppose to work on Big Endian? >> be16_to_cpu(v) is (v) >> and >> __cpu_to_le16(b) is swab16(v) > > Yes. But on native BE system ( I expect that v is in big endian) iowrite16be(v, addr) should be just *(volatile u16 __force *) addr = v; not *(volatile u16 __force *) addr = swab16(v); >> On little >> be16_to_cpu(v) is swab16(v) > > Yes. > >> and >> __cpu_to_le16(swab(b)) is swab16(v) > > ??? > > Don't you mean "__cpu_to_le16(b) is (b)"? BTW: I took output value from the first line (be16_to_cpu(v) is swab16(v)) Grrr - on LE this code works. (I expect that v is in little endian) iowrite16be(v, addr) is *(volatile u16 __force *) addr = swab16(v); >> What I would expect is >> #define iowrite16be(v, addr) __raw_writew(__cpu_to_be16(v), addr) > > Indeed, it should be "__cpu_to_be16(v)" instead of "be16_to_cpu(v)". What do you mean by that? Thanks, Michal -- 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 and responsible for u-boot arm zynq platform -- 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] drivers/block/xsysace - replace in(out)_8/in(out)_be16/in(out)_le16 with generic iowrite(read)8/16(be)
On 02/07/2013 08:38 AM, Geert Uytterhoeven wrote: On Thu, Feb 7, 2013 at 8:23 AM, Michal Simek wrote: #define iowrite16be(v, addr) iowrite16(be16_to_cpu(v), (addr)) #define iowrite16(v, addr) writew((v), (addr)) #define writew(b,addr) __raw_writew(__cpu_to_le16(b),addr) static inline void __raw_writew(u16 b, volatile void __iomem *addr) { *(volatile u16 __force *) addr = b; } How is this suppose to work on Big Endian? be16_to_cpu(v) is (v) and __cpu_to_le16(b) is swab16(v) Yes. But on native BE system ( I expect that v is in big endian) iowrite16be(v, addr) should be just *(volatile u16 __force *) addr = v; not *(volatile u16 __force *) addr = swab16(v); What I would expect is #define iowrite16be(v, addr) __raw_writew(__cpu_to_be16(v), addr) Indeed, it should be "__cpu_to_be16(v)" instead of "be16_to_cpu(v)". What do you mean by that? Bummer, I missed that current iowrite16be() uses (the little endian) iowrite16(), not _raw_writew(), and thought the only difference between the original and your version was the endianness conversion macro. Yes, #define iowrite16be(v, addr) __raw_writew(__cpu_to_be16(v), addr) should be correct. ok. Can you please confirm with me that the same problem is also for iowrite32be ioread16be and ioread32be? This description seems to me correct for BE and LE. #define ioread16be(addr) __be16_to_cpu(__raw_readw(addr)) #define ioread32be(addr) __be32_to_cpu(__raw_readl(addr)) #define iowrite16be(v, addr) __raw_writew(__cpu_to_be16(v), addr) #define iowrite32be(v, addr) __raw_writel(__cpu_to_be32(v), addr) What do you think? Thanks, Michal -- Michal Simek, Ing. (M.Eng) w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/ Microblaze U-BOOT custodian -- 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/
[PATCH] asm-generic: io: Fix ioread16/32be and iowrite16/32be
Fix ioreadXXbe and iowriteXXbe functions which did additional little endian conversion on native big endian systems. Using be_to_cpu (cpu_to_be) conversions with __raw_read/write functions have resolved it. Signed-off-by: Michal Simek CC: Benjamin Herrenschmidt CC: Arnd Bergmann CC: Geert Uytterhoeven CC: Will Deacon CC: linux-a...@vger.kernel.org --- include/asm-generic/io.h |8 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h index 33bbbae..8823581 100644 --- a/include/asm-generic/io.h +++ b/include/asm-generic/io.h @@ -225,15 +225,15 @@ static inline void outsl(unsigned long addr, const void *buffer, int count) #ifndef CONFIG_GENERIC_IOMAP #define ioread8(addr) readb(addr) #define ioread16(addr) readw(addr) -#define ioread16be(addr) be16_to_cpu(ioread16(addr)) +#define ioread16be(addr) __be16_to_cpu(__raw_readw(addr)) #define ioread32(addr) readl(addr) -#define ioread32be(addr) be32_to_cpu(ioread32(addr)) +#define ioread32be(addr) __be32_to_cpu(__raw_readl(addr)) #define iowrite8(v, addr) writeb((v), (addr)) #define iowrite16(v, addr) writew((v), (addr)) -#define iowrite16be(v, addr) iowrite16(be16_to_cpu(v), (addr)) +#define iowrite16be(v, addr) __raw_writew(__cpu_to_be16(v), addr) #define iowrite32(v, addr) writel((v), (addr)) -#define iowrite32be(v, addr) iowrite32(be32_to_cpu(v), (addr)) +#define iowrite32be(v, addr) __raw_writel(__cpu_to_be32(v), addr) #define ioread8_rep(p, dst, count) \ insb((unsigned long) (p), (dst), (count)) -- 1.7.0.4 -- 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] drivers/block/xsysace - replace in(out)_8/in(out)_be16/in(out)_le16 with generic iowrite(read)8/16(be)
2013/2/7 Arnd Bergmann : > On Thursday 07 February 2013, Geert Uytterhoeven wrote: >> >> On Thu, Feb 7, 2013 at 9:01 AM, Michal Simek wrote: >> > ok. Can you please confirm with me that the same problem is also for >> > iowrite32be >> > ioread16be and ioread32be? >> > >> > This description seems to me correct for BE and LE. >> > #define ioread16be(addr) __be16_to_cpu(__raw_readw(addr)) >> > #define ioread32be(addr) __be32_to_cpu(__raw_readl(addr)) >> > >> > #define iowrite16be(v, addr) __raw_writew(__cpu_to_be16(v), addr) >> > #define iowrite32be(v, addr) __raw_writel(__cpu_to_be32(v), addr) >> > >> > What do you think? >> >> Looks fine to me. Arnd, Ben? > > Yes, that would be better. Can you prepare a patch? Done: Subject: "asm-generic: io: Fix ioread16/32be and iowrite16/32be" Thanks, Michal -- 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 and responsible for u-boot arm zynq platform -- 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/
[PATCH] block: xsysace: Fix device name assignment for SystemACE (from "xs`" to "xsa").
From: Graeme Smecher This fixes a bug introduced in 5d10302f46d, where device trees that don't provide the "port-number" attribute are mistakenly assigned the device "xs`". The error check that's supposed to assign a default letter can't succeed, since it tests an unsigned type against a negative return code. Signed-off-by: Graeme Smecher Acked-by: Grant Likely Signed-off-by: Michal Simek --- drivers/block/xsysace.c |3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/drivers/block/xsysace.c b/drivers/block/xsysace.c index 64fd3c0..9462e68 100644 --- a/drivers/block/xsysace.c +++ b/drivers/block/xsysace.c @@ -1161,8 +1161,7 @@ static int ace_probe(struct platform_device *dev) dev_dbg(&dev->dev, "ace_probe(%p)\n", dev); /* device id and bus width */ - of_property_read_u32(dev->dev.of_node, "port-number", &id); - if (id < 0) + if (of_property_read_u32(dev->dev.of_node, "port-number", &id)) id = 0; if (of_find_property(dev->dev.of_node, "8-bit", NULL)) bus_width = ACE_BUS_WIDTH_8; -- 1.7.0.4 -- 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] drivers/block/xsysace - replace in(out)_8/in(out)_be16/in(out)_le16 with generic iowrite(read)8/16(be)
2013/2/7 Alexey Brodkin : > On 02/07/2013 01:35 AM, Benjamin Herrenschmidt wrote: >> >> On Wed, 2013-02-06 at 10:14 +, Grant Likely wrote: >>> >>> >>> Huh? That makes no sense. This device out in the wild with both big >>> and little endian bus attachments. You can argue all day that one of >>> them is wrong, but it doesn't matter. It exists, is used, and must be >>> supported. >> >> >> No. That's where you are VERY wrong. We don't have to support crap and >> arguably shouldn't if that can give an incentive to vendors to fix their >> stuff. If you don't believe me, ask Linus :-) >> >>> In fact, the driver already knows about this and figures >>> out at runtime how the device is wired up to the bus. This is not the >>> problem. >> >> >> Except that this is very gross, especially when you observe that in the >> busted "big endian" case, it has to byteswap the bloody data port. >> >> So you end up having to do that gross hack with separate accessors for >> registers vs. data and not able to use the _rep variants, which also >> means that on platforms like ppc, you end up with a memory barrier on >> every access (or more), which is going to slow things down enormously. > > > BTW I've just realized that in case if there's no bridge between CPU and > CF-controller or if this bridge is "transparent" (does no swapping > neither bytes nor bits) our data accessors here should be changed. > > Isn't it strange in "ace_datain_le16" use "ioread16be" or the one it was > here initially "in_be16"? > With BE ones I'd say similar changes should be done. > > So finally I see them implemented this way: > === > /* BE part */ > > static void ace_datain_be16(struct ace_device *ace) > { > int i = ACE_FIFO_SIZE / 2; > u16 *dst = ace->data_ptr; > while (i--) > *dst++ = ioread16be(ace->baseaddr + 0x40); > ace->data_ptr = dst; > } > > static void ace_dataout_be16(struct ace_device *ace) > { > int i = ACE_FIFO_SIZE / 2; > u16 *src = ace->data_ptr; > while (i--) > iowrite16be(*src++, ace->baseaddr + 0x40); > ace->data_ptr = src; > } > > /* LE part*/ > > static void ace_datain_le16(struct ace_device *ace) > { > int i = ACE_FIFO_SIZE / 2; > u16 *dst = ace->data_ptr; > while (i--) > *dst++ = ioread16(ace->baseaddr + 0x40); > ace->data_ptr = dst; > } > > static void ace_dataout_le16(struct ace_device *ace) > { > int i = ACE_FIFO_SIZE / 2; > u16 *src = ace->data_ptr; > while (i--) > iowrite16(*src++, ace->baseaddr + 0x40); > ace->data_ptr = src; > } > === > > Correct me if I'm wrong here. > > And at least these accessors for LE got xsysace perfectly working on our > FPGA platform (little-endian ARC700 on Xilinx ml-509 with our own > BVCI-to-MPU bridge that does no swapping). > > I have to confess that I didn't properly tested initial patch on real HW - > it was only sort of cosmetic clean-up. The origin patch (after some microblaze ioread/iowrite fixes) works ok, These additional changes is breaking sysace on microblaze big endian ml505 16bit mode. 8bit mode just works Also I have looked at our tree and I see that the mainline kernel misses one patch which was sent by Graeme Smecher and it was also Acked-by Grant. It is called "Fix device name assignment for SystemACE (from "xs`" to "xsa")." Let me resend it too. Alexey: Can you please confirm that on your arc platform you see broken CF partitions name? Thanks, Michal -- 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 and responsible for u-boot arm zynq platform -- 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] asm-generic: io: Fix ioread16/32be and iowrite16/32be
2013/2/8 Geert Uytterhoeven : > On Thu, Feb 7, 2013 at 3:18 PM, Michal Simek wrote: >> Fix ioreadXXbe and iowriteXXbe functions which did >> additional little endian conversion on native big endian systems. >> Using be_to_cpu (cpu_to_be) conversions with __raw_read/write >> functions have resolved it. >> >> Signed-off-by: Michal Simek >> CC: Benjamin Herrenschmidt >> CC: Arnd Bergmann >> CC: Geert Uytterhoeven >> CC: Will Deacon >> CC: linux-a...@vger.kernel.org > > I have one question (see below). Apart from that: > Acked-by: Geert Uytterhoeven > >> -#define ioread16be(addr) be16_to_cpu(ioread16(addr)) >> +#define ioread16be(addr) __be16_to_cpu(__raw_readw(addr)) > > Why did you change it to the __beX_to_cpu variant with underscores? The question could be probably different. Why are they even defined? I have grepped the kernel and all archs use these generic macros include/linux/byteorder/generic.h:94:#define be32_to_cpu __be32_to_cpu include/linux/byteorder/generic.h:106:#define be32_to_cpup __be32_to_cpup include/linux/byteorder/generic.h:118:#define be32_to_cpus __be32_to_cpus What about to remove them? Back to you question. I can't remember particular reason for that maybe just experience that __ versions should be the fastest kernel implementation. Thanks, Michal -- 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 and responsible for u-boot arm zynq platform -- 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] drivers/block/xsysace - replace in(out)_8/in(out)_be16/in(out)_le16 with generic iowrite(read)8/16(be)
2013/2/8 Arnd Bergmann : > On Thursday 07 February 2013, Michal Simek wrote: >> Subject: "asm-generic: io: Fix ioread16/32be and iowrite16/32be" > > Ok, thanks. If you don't mind, I think this can just go with the other patches > for xsysace that come out of this discussion. I want to move microblaze to use this asm-generic/io.h that's why I prefer to send this with microblaze patch directly to Linus. For me this make more sense that with sending block device driver changes. What do you think? Thanks, Michal -- 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 and responsible for u-boot arm zynq platform -- 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] drivers/block/xsysace - replace in(out)_8/in(out)_be16/in(out)_le16 with generic iowrite(read)8/16(be)
2013/2/11 Arnd Bergmann : > On Monday 11 February 2013, Michal Simek wrote: >> >> 2013/2/8 Arnd Bergmann : >> > On Thursday 07 February 2013, Michal Simek wrote: >> >> Subject: "asm-generic: io: Fix ioread16/32be and iowrite16/32be" >> > >> > Ok, thanks. If you don't mind, I think this can just go with the other >> > patches >> > for xsysace that come out of this discussion. >> >> I want to move microblaze to use this asm-generic/io.h that's why I >> prefer to send >> this with microblaze patch directly to Linus. For me this make more sense >> that >> with sending block device driver changes. >> >> What do you think? > > Sure, please do it that way. My policy for asm-generic patches is that they > I prefer to have them go through whatever tree needs them, I was just confused > about which one that would be in this case. > I have just found that it won't be so easy as I thought because I have found that microblaze wrong implementation was done because of others device drivers. It means that I have to fix all device drivers to support big and little endian accessors first before I can switch microblaze to asm-generic/io.h. :-( BTW: If you want to take this patch though your tree then ok, or it can go through my microblaze tree. Both ways should just work. I will send the patch for uarlite when I finish my testing on microblaze, ppc and arm. Thanks, Michal -- 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 and responsible for u-boot arm zynq platform -- 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] drivers/block/xsysace - replace in(out)_8/in(out)_be16/in(out)_le16 with generic iowrite(read)8/16(be)
2013/2/11 Arnd Bergmann : > On Monday 11 February 2013, Michal Simek wrote: >> I have just found that it won't be so easy as I thought because I have found >> that microblaze wrong implementation was done because of others device >> drivers. >> It means that I have to fix all device drivers to support big and >> little endian accessors >> first before I can switch microblaze to asm-generic/io.h. :-( >> >> BTW: If you want to take this patch though your tree then ok, or it >> can go through my >> microblaze tree. Both ways should just work. > > Please use your tree then. I don't have any asm-generic patches > lined up for 3.9 myself, so we can save me and Linus a little work > with a trivial pull request that way. > >> I will send the patch for uarlite when I finish my testing on >> microblaze, ppc and arm. > > Ok. Is that the only one that you found to require the "wrong-endian" mode > in microblaze. Unfortunately no. Another is spi/i2c (sysace as we discuss in this thread), probably icap network drivers are ok because they are not shared. Timer when it is moved to clocksource(not important right now) xilinx gpio is using __raw IO functions which is incorrect on arm in connection to barriers. But it reminds me that maybe the easiest solution is not to use endian accessors just use two simple macros which should work on all systems. #define _readreg(offset) ({__raw_readl(offset); rmb(); }) #define _writereg(offset, val)({wmb(); __raw_writel(val, offset); }) which is probably the faster solution which add minimum additional code to driver and can also remove endian detection code. Thanks, Michal -- 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 and responsible for u-boot arm zynq platform -- 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/
[PATCH 1/2] tty: serial: uartlite: Fix sparse and checkpatch warnings
Clean coding style and sparse warnings. Signed-off-by: Michal Simek --- drivers/tty/serial/uartlite.c |8 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/tty/serial/uartlite.c b/drivers/tty/serial/uartlite.c index 89eee43..31444be 100644 --- a/drivers/tty/serial/uartlite.c +++ b/drivers/tty/serial/uartlite.c @@ -19,7 +19,7 @@ #include #include #include -#include +#include #include #include #include @@ -615,7 +615,7 @@ static struct platform_driver ulite_platform_driver = { * Module setup/teardown */ -int __init ulite_init(void) +static int __init ulite_init(void) { int ret; @@ -634,11 +634,11 @@ int __init ulite_init(void) err_plat: uart_unregister_driver(&ulite_uart_driver); err_uart: - printk(KERN_ERR "registering uartlite driver failed: err=%i", ret); + pr_err("registering uartlite driver failed: err=%i", ret); return ret; } -void __exit ulite_exit(void) +static void __exit ulite_exit(void) { platform_driver_unregister(&ulite_platform_driver); uart_unregister_driver(&ulite_uart_driver); -- 1.7.0.4 -- 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/
[PATCH 2/2] tty: serial: uartlite: Support uartlite on big and little endian systems
Use big and little endian accessors function to reflect system configuration. Detection is done via control register in ulite_request_port. Tested on Microblaze LE, BE, PPC440 and Arm zynq. Signed-off-by: Michal Simek --- drivers/tty/serial/uartlite.c | 101 - 1 files changed, 79 insertions(+), 22 deletions(-) diff --git a/drivers/tty/serial/uartlite.c b/drivers/tty/serial/uartlite.c index 31444be..59be23b 100644 --- a/drivers/tty/serial/uartlite.c +++ b/drivers/tty/serial/uartlite.c @@ -34,7 +34,7 @@ * Register definitions * * For register details see datasheet: - * http://www.xilinx.com/support/documentation/ip_documentation/opb_uartlite.pdf + * http://www.xilinx.com/support/documentation/ip_documentation/opb_uartlite.pdf */ #define ULITE_RX 0x00 @@ -57,6 +57,54 @@ #define ULITE_CONTROL_RST_RX 0x02 #define ULITE_CONTROL_IE 0x10 +struct uartlite_reg_ops { + u32 (*in)(void __iomem *addr); + void (*out)(u32 val, void __iomem *addr); +}; + +static u32 uartlite_inbe32(void __iomem *addr) +{ + return ioread32be(addr); +} + +static void uartlite_outbe32(u32 val, void __iomem *addr) +{ + iowrite32be(val, addr); +} + +static struct uartlite_reg_ops uartlite_be = { + .in = uartlite_inbe32, + .out = uartlite_outbe32, +}; + +static u32 uartlite_inle32(void __iomem *addr) +{ + return ioread32(addr); +} + +static void uartlite_outle32(u32 val, void __iomem *addr) +{ + iowrite32(val, addr); +} + +static struct uartlite_reg_ops uartlite_le = { + .in = uartlite_inle32, + .out = uartlite_outle32, +}; + +static inline u32 uart_in32(u32 offset, struct uart_port *port) +{ + struct uartlite_reg_ops *reg_ops = port->private_data; + + return reg_ops->in(port->membase + offset); +} + +static inline void uart_out32(u32 val, u32 offset, struct uart_port *port) +{ + struct uartlite_reg_ops *reg_ops = port->private_data; + + reg_ops->out(val, port->membase + offset); +} static struct uart_port ulite_ports[ULITE_NR_UARTS]; @@ -77,7 +125,7 @@ static int ulite_receive(struct uart_port *port, int stat) /* stats */ if (stat & ULITE_STATUS_RXVALID) { port->icount.rx++; - ch = ioread32be(port->membase + ULITE_RX); + ch = uart_in32(ULITE_RX, port); if (stat & ULITE_STATUS_PARITY) port->icount.parity++; @@ -122,7 +170,7 @@ static int ulite_transmit(struct uart_port *port, int stat) return 0; if (port->x_char) { - iowrite32be(port->x_char, port->membase + ULITE_TX); + uart_out32(port->x_char, ULITE_TX, port); port->x_char = 0; port->icount.tx++; return 1; @@ -131,7 +179,7 @@ static int ulite_transmit(struct uart_port *port, int stat) if (uart_circ_empty(xmit) || uart_tx_stopped(port)) return 0; - iowrite32be(xmit->buf[xmit->tail], port->membase + ULITE_TX); + uart_out32(xmit->buf[xmit->tail], ULITE_TX, port); xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE-1); port->icount.tx++; @@ -148,7 +196,7 @@ static irqreturn_t ulite_isr(int irq, void *dev_id) int busy, n = 0; do { - int stat = ioread32be(port->membase + ULITE_STATUS); + int stat = uart_in32(ULITE_STATUS, port); busy = ulite_receive(port, stat); busy |= ulite_transmit(port, stat); n++; @@ -169,7 +217,7 @@ static unsigned int ulite_tx_empty(struct uart_port *port) unsigned int ret; spin_lock_irqsave(&port->lock, flags); - ret = ioread32be(port->membase + ULITE_STATUS); + ret = uart_in32(ULITE_STATUS, port); spin_unlock_irqrestore(&port->lock, flags); return ret & ULITE_STATUS_TXEMPTY ? TIOCSER_TEMT : 0; @@ -192,7 +240,7 @@ static void ulite_stop_tx(struct uart_port *port) static void ulite_start_tx(struct uart_port *port) { - ulite_transmit(port, ioread32be(port->membase + ULITE_STATUS)); + ulite_transmit(port, uart_in32(ULITE_STATUS, port)); } static void ulite_stop_rx(struct uart_port *port) @@ -220,17 +268,17 @@ static int ulite_startup(struct uart_port *port) if (ret) return ret; - iowrite32be(ULITE_CONTROL_RST_RX | ULITE_CONTROL_RST_TX, - port->membase + ULITE_CONTROL); - iowrite32be(ULITE_CONTROL_IE, port->membase + ULITE_CONTROL); + uart_out32(ULITE_CONTROL_RST_RX | ULITE_CONTROL_RST_TX, + ULITE_CONTROL, port); + uart_out32(ULITE_CONTROL_IE, ULITE_CONTROL, port); return 0; } static void ulite_shutdown(struct uart_port *port) { - iowrite32be(0, port->membase + ULITE_CONTROL); -
Re: [PATCH] drivers/block/xsysace - replace in(out)_8/in(out)_be16/in(out)_le16 with generic iowrite(read)8/16(be)
2013/2/12 Benjamin Herrenschmidt : > On Mon, 2013-02-11 at 16:57 +0100, Michal Simek wrote: >> But it reminds me that maybe the easiest solution is not to use endian >> accessors just use two simple macros which should work on all systems. >> >> #define _readreg(offset) ({__raw_readl(offset); rmb(); }) >> #define _writereg(offset, val)({wmb(); __raw_writel(val, offset); >> }) >> >> which is probably the faster solution which add minimum additional code >> to driver and can also remove endian detection code. > > Except that rmb & wmb might not be the right barriers for IOs ... Arm is using __iormb in readX macros #define __iormb() rmb() but it is arm specific only that's why I have added there rmb(). Thanks, Michal -- 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 and responsible for u-boot arm zynq platform -- 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] drivers/block/xsysace - replace in(out)_8/in(out)_be16/in(out)_le16 with generic iowrite(read)8/16(be)
2013/2/11 Arnd Bergmann : > On Monday 11 February 2013, Michal Simek wrote: >> Unfortunately no. Another is spi/i2c (sysace as we discuss in this >> thread), probably icap >> network drivers are ok because they are not shared. >> Timer when it is moved to clocksource(not important right now) >> xilinx gpio is using __raw IO functions which is incorrect on arm in >> connection to barriers. >> >> But it reminds me that maybe the easiest solution is not to use endian >> accessors just use two simple macros which should work on all systems. >> >> #define _readreg(offset) ({__raw_readl(offset); rmb(); }) >> #define _writereg(offset, val)({wmb(); __raw_writel(val, offset); >> }) >> >> which is probably the faster solution which add minimum additional code >> to driver and can also remove endian detection code. > > I tend to disagree. You should never assume that a device is the same > endianess as the the CPU, and you should try not to use the __raw_* > accessors in device drivers either. > > In particular, ARM can run both big- and little-endian even though > big-endian is rarely used, so you need to know the endianess for > the device you are talking to rather than assume that it knows > what the CPU does at the time. For high performance IPs using accessors functions is still problematic because there will be performance regression it means that from my point of view there still should be any option to "setup" proper endians for the driver and it can't be setup at run-time. Sure the question is if drivers like this should be in the mainline. But if yes, there should be an option to do it in acceptable way. Thanks, Michal -- 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 and responsible for u-boot arm zynq platform -- 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] drivers/block/xsysace - replace in(out)_8/in(out)_be16/in(out)_le16 with generic iowrite(read)8/16(be)
2013/2/12 Arnd Bergmann : > On Tuesday 12 February 2013, Michal Simek wrote: >> > In particular, ARM can run both big- and little-endian even though >> > big-endian is rarely used, so you need to know the endianess for >> > the device you are talking to rather than assume that it knows >> > what the CPU does at the time. >> >> For high performance IPs using accessors functions is still problematic >> because there will be performance regression it means that >> from my point of view there still should be any option to "setup" >> proper endians for the driver and it can't be setup at run-time. > > I did not mean you have to use a run-time detection here, although > that is often the easiest solution. If you know the endianess of > a device for a specific architecture or platform, it is totally > fine to pick that endianess at compile-time and use e.g. the > readl_relaxed() accessors on ARM to give you the lowest access > latency. ok but still there should be well defined how to do it. Let's say generic Kconfig option. Or maybe there is also third option to modified code to use proper writes. Thanks, Michal -- 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 and responsible for u-boot arm zynq platform -- 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] drivers/block/xsysace - replace in(out)_8/in(out)_be16/in(out)_le16 with generic iowrite(read)8/16(be)
2013/2/7 Grant Likely : > On Thu, Feb 7, 2013 at 3:12 PM, Alexey Brodkin > wrote: >> On 02/07/2013 06:51 PM, Grant Likely wrote: >>> >>> On Thu, Feb 7, 2013 at 2:39 PM, Grant Likely >>> wrote: >>>> >>>> On Wed, Feb 6, 2013 at 9:35 PM, Benjamin Herrenschmidt >>>> wrote: >>>>>> >>>>>> In fact, the driver already knows about this and figures >>>>>> out at runtime how the device is wired up to the bus. This is not the >>>>>> problem. >>>>> >>>>> >>>>> Except that this is very gross, especially when you observe that in the >>>>> busted "big endian" case, it has to byteswap the bloody data port. >>>>> >>>>> So you end up having to do that gross hack with separate accessors for >>>>> registers vs. data and not able to use the _rep variants, which also >>>>> means that on platforms like ppc, you end up with a memory barrier on >>>>> every access (or more), which is going to slow things down enormously. >>>> >>>> >>>> I don't see why the _rep variants aren't usable here. The only reason >>>> I didn't use them when I wrote the driver in the first place was I was >>>> a n00b kernel hacker and I didn't know they were there. >>> >>> >>> The 8-bit variant is different though because the hardware requires >>> pingponging between odd and even byte addresses to flush the fifo. >>> Reading a data port even address (0x40) gives the least significant >>> byte. Reading from an odd address (0x41) give the MSB and pops the >>> data off the FIFO. So, yes, the _rep variant can't be used in 8-bit >>> mode. It should still be fine in 16-bit. >>> >>> page 45: http://www.xilinx.com/support/documentation/data_sheets/ds080.pdf >>> >> >> Ok, so may I do a re-spin with these changes: > > There are two things here. 1) changing the accessors used. 2) > switching the endianess as a bug fix. Any changes to the endian access > should be a separate patch which a description of what is needed. > >> 1. In "ace_in_be16" use "ioread16be" >> 2. In "ace_out_be16" use "iowrite16be" >> 3. In "ace_in_le16" use "ioread16" >> 4. In "ace_out_le16" use "iowrite16" > > Yes > >> 5. In "ace_datain_le16" use "ioread16_rep" >> 6. In "ace_dataout_le16" use "iowrite16_rep" > > Maybe. In a separate patch. Hmmm... I guess there isn't an > ioread16be_rep variant. Oh well. Check first with Michal on LE > microblaze before making the change. If it doesn't work for him the > more understanding is needed. I was pretty sure the LE variant already > worked. Sorry it took me some time to get 16bit LE to work for test but here are test results. I have tested it on ppc BE, Microblaze BE and Microblaze LE systems and surprisingly on all of them only ace_reg_le16_ops are used. But on Microblaze LE is necessary to use different datain/out_le16 functions as below which are also not compatible with Microblaze and PPC BE. diff --git a/drivers/block/xsysace.c b/drivers/block/xsysace.c index bbad046..8dd192c 100644 --- a/drivers/block/xsysace.c +++ b/drivers/block/xsysace.c @@ -314,7 +314,7 @@ static void ace_datain_le16(struct ace_device *ace) int i = ACE_FIFO_SIZE / 2; u16 *dst = ace->data_ptr; while (i--) - *dst++ = ioread16be(ace->baseaddr + 0x40); + *dst++ = ioread16(ace->baseaddr + 0x40); ace->data_ptr = dst; } @@ -323,7 +323,7 @@ static void ace_dataout_le16(struct ace_device *ace) int i = ACE_FIFO_SIZE / 2; u16 *src = ace->data_ptr; while (i--) - iowrite16be(*src++, ace->baseaddr + 0x40); + iowrite16(*src++, ace->baseaddr + 0x40); ace->data_ptr = src; } Grant: How did you tested BE mode? It is fpga it means you can switch some connections and it could work. Thanks, Michal -- 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 and responsible for u-boot arm zynq platform -- 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: linux-next: manual merge of the arm-soc tree with the xilinx tree
On 04/05/2013 03:32 PM, Arnd Bergmann wrote: On Friday 05 April 2013, Michal Simek wrote: Interesting. I rebased my arm-next branch based on 3.9-rc5 with some Rob's + one Arnd patch from arm-soc - clksrc/cleanup branch. I will fix my arm-next branch. The for-next branch in arm-soc is not stable, you should never base anything on it. If you depend on some stable branch, that is in arm-soc, then use just that branch, not one of the next/* branches or for-next. I haven't based on arm-soc for-next branch my arm-next branch. I just took all patches I need for zynq and done git rebase v3.5-rc5. Which caused that I have became commuter of that 4 patches and there is probably any conflict between your for-next branch and clksrc/cleanup which you have resolved in for-next branch. Ah, I see. That was actually my fault, I'm sorry for causing trouble here and then accusing you instead. And because of my rebase sha1 are different that's why Stephen had problem with it. I have changed my arm-next branch and will see on Monday if Stephen will report any problem or not. Ok, thanks! No worries. Thanks, Michal -- Michal Simek, Ing. (M.Eng) w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/ Microblaze U-BOOT custodian -- 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] drivers/block/xsysace - replace in(out)_8/in(out)_be16/in(out)_le16 with generic iowrite(read)8/16(be)
2013/1/30 Alexey Brodkin : > On 01/29/2013 08:27 PM, Arnd Bergmann wrote: >> >> On Tuesday 29 January 2013, Alexey Brodkin wrote: >>> >>> in(out)_8/in(out)_be16/in(out)_le16 are very powerpc/microblaze >>> specific. To enable use of Xilinx System ACE driver build for other >>> architectures (for example it's possible to use it on Xilinx ml-509 >>> board with ARC700 in FPGA) we need to use generic implementation of >>> accessors. >>> >>> Current implementation was successfully built with Sourcery G++ Lite >>> 2011.03-39 for Power EABI (ppc44x_defconfig). >>> >>> Signed-off-by: Alexey Brodkin >> >> >> Is this driver used on powerpc64 as well, or just on microblaze >> and/or 32 bit powerpc? >> >> On 64 bit powerpc, ioread involves extra overhead because it >> goes through the PCI error handling implementation, so we should >> keep using in_le() there. >> >> Arnd >> > > Personally I have no idea about usage of the named device on powerpc64. > Wondering if anybody may comment on this? > > My only intention was to make the driver portable. > Do you think if there's another generic alternative for originally used > accessors? > > For example will it be better with "readb/readw/writeb/writew"? This driver is used on ppc32, microblaze little and big endian and probably could be used on arm le. Not sure if we can find out proper IO function which we should use. btw: I will have to support all these combinations for uartlite, xilinx_spi, gpio and others xilinx IPs. Thanks, Michal -- 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 and responsible for u-boot arm zynq platform -- 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] drivers/block/xsysace - replace in(out)_8/in(out)_be16/in(out)_le16 with generic iowrite(read)8/16(be)
2013/1/30 Vineet Gupta : > On Wednesday 30 January 2013 04:43 PM, Michal Simek wrote: >> 2013/1/30 Alexey Brodkin : >>> On 01/29/2013 08:27 PM, Arnd Bergmann wrote: >>>> On Tuesday 29 January 2013, Alexey Brodkin wrote: >>>>> in(out)_8/in(out)_be16/in(out)_le16 are very powerpc/microblaze >>>>> specific. To enable use of Xilinx System ACE driver build for other >>>>> architectures (for example it's possible to use it on Xilinx ml-509 >>>>> board with ARC700 in FPGA) we need to use generic implementation of >>>>> accessors. >>>>> >>>>> Current implementation was successfully built with Sourcery G++ Lite >>>>> 2011.03-39 for Power EABI (ppc44x_defconfig). >>>>> >>>>> Signed-off-by: Alexey Brodkin >>>> >>>> Is this driver used on powerpc64 as well, or just on microblaze >>>> and/or 32 bit powerpc? >>>> >>>> On 64 bit powerpc, ioread involves extra overhead because it >>>> goes through the PCI error handling implementation, so we should >>>> keep using in_le() there. >>>> >>>> Arnd >>>> >>> Personally I have no idea about usage of the named device on powerpc64. >>> Wondering if anybody may comment on this? >>> >>> My only intention was to make the driver portable. >>> Do you think if there's another generic alternative for originally used >>> accessors? >>> >>> For example will it be better with "readb/readw/writeb/writew"? >> This driver is used on ppc32, microblaze little and big endian >> and probably could be used on arm le. > > We intend to use it on ARC architecture, for the Xilinx ML509 boards we use. > ARC > port doesn't yet have the special I/O accessors this driver uses. While I can > easily add them to ARC port, it would be better to make this driver portable. > This > either means replacing the special I/O accessors the driver uses with what we > have > now (if possible on arches) or else provide a default implementation for these > special accessors in asm-generic. I'm up for either one although first > approach is > preferable as it avoid adding another set to the zoo of accessors we have > right now. > > But just adding the accessors to ARC port is a band-aid IMHO. > >> Not sure if we can find out proper IO function which we should use. > > Is it because of the intricacies of I/O on the arches it uses or because we > don't > have well defined semantics of what read/ioread friends should do - or yet > something else. The problem is mainly for ppc and arm. (mb is ok because of little endian implementation) In the driver for arm you can't simple use ioread16be because it should be little endian function. And vice versa. Also from my understanding of arm we should use readl/b/w functions because they have memory barriers which should be probably performed. And I haven't found any IO function which will behave on arm as LE and on PPC as BE. Maybe you have some suggestion how to solve it. Look at this thread "Using IO functions across ARM, PPC and Microblaze architectures" Thanks, Michal -- 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 and responsible for u-boot arm zynq platform -- 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: Using IO functions across ARM, PPC and Microblaze architectures
On 12/08/2012 11:57 AM, Geert Uytterhoeven wrote: On Fri, Dec 7, 2012 at 10:16 PM, Alan Cox wrote: Because I need to use IO functions which will behave on arm as little endian and on powerpc as big endian and on microblaze depends on endian setting. I haven't found any IO function which I could use by 3 architectures without using preprocessor macros or runtime detection Its a rather weird mix. We can do "always big" and "always little" 1. Using helper function + preprocessor macros (using static inline function also possible) Then someone comes along and sticks a daughterboard into the system with the same device the other way around and there are years 2. Using function pointers Probably smarter. 8250.c works this way and it has to handle some extremely bizarre mappings. b) Runtime initialization - here is the question if there is any standard function which I could use. Set the pointers up and pass them as data with your platform device, that way the function definitions are buried in your platform code where they depend. Or embed a struct io_ops * in struct device, to be set up by the bus driver? Wasn't David Hinds working on something like this in the context of PCMCIA a few decades ago? Do you have any link on that? Thanks, Michal -- Michal Simek, Ing. (M.Eng) w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/ Microblaze U-BOOT custodian -- 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/
[GIT PULL] arch/microblaze changes for 3.9
Hi Linus, please pull these microblaze changes to your tree. After my discussion with Arnd I have also added there asm-generic io patch which is Acked by him and Geert. Thanks, Michal The following changes since commit 836dc9e3fbbab0c30aa6e664417225f5c1fb1c39: Linux 3.8-rc7 (2013-02-09 08:20:39 +1100) are available in the git repository at: git://git.monstr.eu/linux-2.6-microblaze.git next for you to fetch changes up to 711e5b4520986380700e6f095608021cf087170e: asm-generic: io: Fix ioread16/32be and iowrite16/32be (2013-02-12 11:29:46 +0100) Jason Wu (1): microblaze: Makefile clean Lars-Peter Clausen (1): microblaze: Add .gitignore entries for auto-generated files Michal Simek (5): microblaze: Fix strncpy_from_user macro microblaze: Add missing return from debugfs_tlb microblaze: Fix coding style issues microblaze: Do not use module.h in files which are not modules asm-generic: io: Fix ioread16/32be and iowrite16/32be arch/microblaze/Makefile |9 +- arch/microblaze/boot/.gitignore |3 + arch/microblaze/include/asm/io.h |2 +- arch/microblaze/kernel/.gitignore |1 + arch/microblaze/kernel/cpu/cache.c| 148 - arch/microblaze/kernel/cpu/cpuinfo-pvr-full.c | 21 ++-- arch/microblaze/kernel/cpu/cpuinfo.c | 13 +-- arch/microblaze/kernel/cpu/pvr.c |2 +- arch/microblaze/kernel/dma.c |6 +- arch/microblaze/kernel/early_printk.c | 17 ++- arch/microblaze/kernel/exceptions.c | 27 ++--- arch/microblaze/kernel/ftrace.c | 44 arch/microblaze/kernel/heartbeat.c|2 +- arch/microblaze/kernel/intc.c |4 +- arch/microblaze/kernel/kgdb.c |2 +- arch/microblaze/kernel/microblaze_ksyms.c |2 +- arch/microblaze/kernel/module.c |5 +- arch/microblaze/kernel/process.c | 24 ++-- arch/microblaze/kernel/prom.c |2 +- arch/microblaze/kernel/prom_parse.c |2 +- arch/microblaze/kernel/ptrace.c |2 +- arch/microblaze/kernel/setup.c| 30 ++--- arch/microblaze/kernel/signal.c |6 +- arch/microblaze/kernel/stacktrace.c |2 +- arch/microblaze/kernel/sys_microblaze.c |3 +- arch/microblaze/kernel/traps.c|8 +- arch/microblaze/kernel/unwind.c |2 +- arch/microblaze/lib/ashldi3.c |3 +- arch/microblaze/lib/ashrdi3.c |3 +- arch/microblaze/lib/cmpdi2.c |2 +- arch/microblaze/lib/lshrdi3.c |3 +- arch/microblaze/lib/memcpy.c | 12 +- arch/microblaze/lib/memmove.c | 11 +- arch/microblaze/lib/memset.c |2 +- arch/microblaze/lib/muldi3.c |2 +- arch/microblaze/lib/uaccess_old.S |9 +- arch/microblaze/lib/ucmpdi2.c |2 +- arch/microblaze/mm/consistent.c |7 +- arch/microblaze/mm/fault.c| 10 +- arch/microblaze/mm/highmem.c |2 +- arch/microblaze/mm/init.c | 32 +++--- arch/microblaze/mm/pgtable.c | 17 ++- arch/microblaze/pci/indirect_pci.c|2 +- arch/microblaze/pci/iomap.c |2 +- arch/microblaze/pci/pci-common.c | 96 arch/microblaze/pci/xilinx_pci.c | 16 +-- include/asm-generic/io.h |8 +- 47 files changed, 299 insertions(+), 331 deletions(-) create mode 100644 arch/microblaze/boot/.gitignore create mode 100644 arch/microblaze/kernel/.gitignore -- 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 and responsible for u-boot arm zynq platform -- 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 -next] microblaze/PCI: remove duplicated include from pci-common.c
2013/3/11 Wei Yongjun : > From: Wei Yongjun > > Remove duplicated include. > > Signed-off-by: Wei Yongjun > --- > arch/microblaze/pci/pci-common.c | 1 - > 1 file changed, 1 deletion(-) Applied. Thanks, Michal -- 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 and responsible for u-boot arm zynq platform -- 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/
[PATCH] mtd: m25p80: Add support for Numonyx N25Q128
From: Wendy Liang Add the id and sector mappings for SPI flash chips. Signed-off-by: Wendy Liang Signed-off-by: Michal Simek CC: David Woodhouse CC: Artem Bityutskiy CC: Mike Frysinger CC: linux-...@lists.infradead.org --- drivers/mtd/devices/m25p80.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c index 5d0d68c..cad89f1 100644 --- a/drivers/mtd/devices/m25p80.c +++ b/drivers/mtd/devices/m25p80.c @@ -722,6 +722,9 @@ static const struct spi_device_id m25p_ids[] = { { "m25px32-s1", INFO(0x206316, 0, 64 * 1024, 64, SECT_4K) }, { "m25px64",INFO(0x207117, 0, 64 * 1024, 128, 0) }, + /* Numonyx -- n25q */ + { "n25q128",INFO(0x20ba18, 0, 64 * 1024, 256, 0) }, + /* Winbond -- w25x "blocks" are 64K, "sectors" are 4KiB */ { "w25x10", INFO(0xef3011, 0, 64 * 1024, 2, SECT_4K) }, { "w25x20", INFO(0xef3012, 0, 64 * 1024, 4, SECT_4K) }, -- 1.7.0.4 -- 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/
[PATCH 01/11] net: axienet: Remove NETIF_F_SG dropping for no checksum feature
Warning message: eth0: Dropping NETIF_F_SG since no checksum feature. Signed-off-by: Michal Simek CC: Anirudha Sarangi CC: John Linn CC: Grant Likely CC: Rob Herring CC: David S. Miller --- drivers/net/ethernet/xilinx/xilinx_axienet_main.c |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index 0793299..50167ab 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -1494,7 +1494,7 @@ static int __devinit axienet_of_probe(struct platform_device *op) SET_NETDEV_DEV(ndev, &op->dev); ndev->flags &= ~IFF_MULTICAST; /* clear multicast */ - ndev->features = NETIF_F_SG | NETIF_F_FRAGLIST; + ndev->features = NETIF_F_FRAGLIST; ndev->netdev_ops = &axienet_netdev_ops; ndev->ethtool_ops = &axienet_ethtool_ops; @@ -1519,14 +1519,14 @@ static int __devinit axienet_of_probe(struct platform_device *op) XAE_FEATURE_PARTIAL_TX_CSUM; lp->features |= XAE_FEATURE_PARTIAL_TX_CSUM; /* Can checksum TCP/UDP over IPv4. */ - ndev->features |= NETIF_F_IP_CSUM; + ndev->features |= NETIF_F_IP_CSUM | NETIF_F_SG; break; case 2: lp->csum_offload_on_tx_path = XAE_FEATURE_FULL_TX_CSUM; lp->features |= XAE_FEATURE_FULL_TX_CSUM; /* Can checksum TCP/UDP over IPv4. */ - ndev->features |= NETIF_F_IP_CSUM; + ndev->features |= NETIF_F_IP_CSUM | NETIF_F_SG; break; default: lp->csum_offload_on_tx_path = XAE_NO_CSUM_OFFLOAD; -- 1.7.0.4 -- 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/
[PATCH 07/11] net: ll_temac: Fix DMA map size bug
DMA allocates skb->len instead of headlen which is used for DMA. The same fix was applied to the axienet driver. Signed-off-by: Michal Simek CC: Anirudha Sarangi CC: John Linn CC: Grant Likely CC: Rob Herring CC: David S. Miller --- drivers/net/ethernet/xilinx/ll_temac_main.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/xilinx/ll_temac_main.c b/drivers/net/ethernet/xilinx/ll_temac_main.c index 482b572..8786d92 100644 --- a/drivers/net/ethernet/xilinx/ll_temac_main.c +++ b/drivers/net/ethernet/xilinx/ll_temac_main.c @@ -710,8 +710,8 @@ static int temac_start_xmit(struct sk_buff *skb, struct net_device *ndev) cur_p->app0 |= STS_CTRL_APP0_SOP; cur_p->len = skb_headlen(skb); - cur_p->phys = dma_map_single(ndev->dev.parent, skb->data, skb->len, -DMA_TO_DEVICE); + cur_p->phys = dma_map_single(ndev->dev.parent, skb->data, + skb_headlen(skb), DMA_TO_DEVICE); cur_p->app4 = (unsigned long)skb; for (ii = 0; ii < num_frag; ii++) { -- 1.7.0.4 -- 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/
[PATCH 11/11] net: xilinx: Show csum in bootlog
Just show current setting in bootlog. Signed-off-by: Michal Simek CC: Anirudha Sarangi CC: John Linn CC: Grant Likely CC: Rob Herring CC: David S. Miller --- drivers/net/ethernet/xilinx/ll_temac_main.c |2 ++ drivers/net/ethernet/xilinx/xilinx_axienet_main.c |2 ++ 2 files changed, 4 insertions(+), 0 deletions(-) diff --git a/drivers/net/ethernet/xilinx/ll_temac_main.c b/drivers/net/ethernet/xilinx/ll_temac_main.c index 58b2869..8c31fcd 100644 --- a/drivers/net/ethernet/xilinx/ll_temac_main.c +++ b/drivers/net/ethernet/xilinx/ll_temac_main.c @@ -1052,12 +1052,14 @@ static int __devinit temac_of_probe(struct platform_device *op) /* Setup checksum offload, but default to off if not specified */ lp->temac_features = 0; p = (__be32 *)of_get_property(op->dev.of_node, "xlnx,txcsum", NULL); + dev_info(&op->dev, "TX_CSUM %d\n", be32_to_cpup(p)); if (p && be32_to_cpu(*p)) { lp->temac_features |= TEMAC_FEATURE_TX_CSUM; /* Can checksum TCP/UDP over IPv4. */ ndev->features |= NETIF_F_IP_CSUM; } p = (__be32 *)of_get_property(op->dev.of_node, "xlnx,rxcsum", NULL); + dev_info(&op->dev, "RX_CSUM %d\n", be32_to_cpup(p)); if (p && be32_to_cpu(*p)) lp->temac_features |= TEMAC_FEATURE_RX_CSUM; diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index 4ef148f..9ea5be6 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -1528,6 +1528,7 @@ static int __devinit axienet_of_probe(struct platform_device *op) lp->features = 0; p = (__be32 *) of_get_property(op->dev.of_node, "xlnx,txcsum", NULL); + dev_info(&op->dev, "TX_CSUM %d\n", be32_to_cpup(p)); if (p) { switch (be32_to_cpup(p)) { case 1: @@ -1549,6 +1550,7 @@ static int __devinit axienet_of_probe(struct platform_device *op) } } p = (__be32 *) of_get_property(op->dev.of_node, "xlnx,rxcsum", NULL); + dev_info(&op->dev, "RX_CSUM %d\n", be32_to_cpup(p)); if (p) { switch (be32_to_cpup(p)) { case 1: -- 1.7.0.4 -- 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/
[PATCH 10/11] net: ll_temac: Simplify xmit
Use one return statement instead of two. Signed-off-by: Michal Simek CC: Anirudha Sarangi CC: John Linn CC: Grant Likely CC: Rob Herring CC: David S. Miller --- drivers/net/ethernet/xilinx/ll_temac_main.c |4 +--- 1 files changed, 1 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/xilinx/ll_temac_main.c b/drivers/net/ethernet/xilinx/ll_temac_main.c index fec42d9..58b2869 100644 --- a/drivers/net/ethernet/xilinx/ll_temac_main.c +++ b/drivers/net/ethernet/xilinx/ll_temac_main.c @@ -690,10 +690,8 @@ static int temac_start_xmit(struct sk_buff *skb, struct net_device *ndev) cur_p = &lp->tx_bd_v[lp->tx_bd_tail]; if (temac_check_tx_bd_space(lp, num_frag)) { - if (!netif_queue_stopped(ndev)) { + if (!netif_queue_stopped(ndev)) netif_stop_queue(ndev); - return NETDEV_TX_BUSY; - } return NETDEV_TX_BUSY; } -- 1.7.0.4 -- 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/
[PATCH 09/11] net: ll_temac: Move frag loading to frag loop
Load frag value when necessary. Signed-off-by: Michal Simek CC: Anirudha Sarangi CC: John Linn CC: Grant Likely CC: Rob Herring CC: David S. Miller --- drivers/net/ethernet/xilinx/ll_temac_main.c |3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/xilinx/ll_temac_main.c b/drivers/net/ethernet/xilinx/ll_temac_main.c index 8bafa15..fec42d9 100644 --- a/drivers/net/ethernet/xilinx/ll_temac_main.c +++ b/drivers/net/ethernet/xilinx/ll_temac_main.c @@ -686,7 +686,6 @@ static int temac_start_xmit(struct sk_buff *skb, struct net_device *ndev) skb_frag_t *frag; num_frag = skb_shinfo(skb)->nr_frags; - frag = &skb_shinfo(skb)->frags[0]; start_p = lp->tx_bd_p + sizeof(*lp->tx_bd_v) * lp->tx_bd_tail; cur_p = &lp->tx_bd_v[lp->tx_bd_tail]; @@ -715,6 +714,7 @@ static int temac_start_xmit(struct sk_buff *skb, struct net_device *ndev) cur_p->app4 = (unsigned long)skb; for (ii = 0; ii < num_frag; ii++) { + frag = &skb_shinfo(skb)->frags[ii]; lp->tx_bd_tail++; if (lp->tx_bd_tail >= TX_BD_NUM) lp->tx_bd_tail = 0; @@ -725,7 +725,6 @@ static int temac_start_xmit(struct sk_buff *skb, struct net_device *ndev) skb_frag_size(frag), DMA_TO_DEVICE); cur_p->len = skb_frag_size(frag); cur_p->app0 = 0; - frag++; } cur_p->app0 |= STS_CTRL_APP0_EOP; -- 1.7.0.4 -- 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/
[PATCH 03/11] net: axienet: Do not use NO_IRQ
NO_IRQ is not longer used by Microblaze. Signed-off-by: Michal Simek CC: Anirudha Sarangi CC: John Linn CC: Grant Likely CC: Rob Herring CC: David S. Miller --- drivers/net/ethernet/xilinx/xilinx_axienet_main.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index a5b41cd..8d1db13 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -1599,7 +1599,7 @@ static int __devinit axienet_of_probe(struct platform_device *op) lp->rx_irq = irq_of_parse_and_map(np, 1); lp->tx_irq = irq_of_parse_and_map(np, 0); of_node_put(np); - if ((lp->rx_irq == NO_IRQ) || (lp->tx_irq == NO_IRQ)) { + if ((!lp->rx_irq) || (!lp->tx_irq)) { dev_err(&op->dev, "could not determine irqs\n"); ret = -ENOMEM; goto err_iounmap_2; -- 1.7.0.4 -- 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/
[PATCH 02/11] net: axienet: Add ioctl support
Allow user to access the MDIO from userspace. Signed-off-by: Michal Simek CC: Anirudha Sarangi CC: John Linn CC: Grant Likely CC: Rob Herring CC: David S. Miller --- drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 15 +++ 1 files changed, 15 insertions(+), 0 deletions(-) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index 50167ab..a5b41cd 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -1053,6 +1053,20 @@ static void axienet_poll_controller(struct net_device *ndev) } #endif +/* Ioctl MII Interface */ +static int axienet_ioctl(struct net_device *dev, struct ifreq *rq, int cmd) +{ + struct axienet_local *priv = netdev_priv(dev); + + if (!netif_running(dev)) + return -EINVAL; + + if (!priv->phy_dev) + return -ENODEV; + + return phy_mii_ioctl(priv->phy_dev, rq, cmd); +} + static const struct net_device_ops axienet_netdev_ops = { .ndo_open = axienet_open, .ndo_stop = axienet_stop, @@ -1061,6 +1075,7 @@ static const struct net_device_ops axienet_netdev_ops = { .ndo_set_mac_address = netdev_set_mac_address, .ndo_validate_addr = eth_validate_addr, .ndo_set_rx_mode = axienet_set_multicast_list, + .ndo_do_ioctl = axienet_ioctl, #ifdef CONFIG_NET_POLL_CONTROLLER .ndo_poll_controller = axienet_poll_controller, #endif -- 1.7.0.4 -- 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/
[PATCH 08/11] net: ll_temac: Do not use fixed mtu size
Use max mtu instead. Signed-off-by: Michal Simek CC: Anirudha Sarangi CC: John Linn CC: Grant Likely CC: Rob Herring CC: David S. Miller --- drivers/net/ethernet/xilinx/ll_temac_main.c | 14 +++--- 1 files changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/net/ethernet/xilinx/ll_temac_main.c b/drivers/net/ethernet/xilinx/ll_temac_main.c index 8786d92..8bafa15 100644 --- a/drivers/net/ethernet/xilinx/ll_temac_main.c +++ b/drivers/net/ethernet/xilinx/ll_temac_main.c @@ -212,7 +212,7 @@ static void temac_dma_bd_release(struct net_device *ndev) break; else { dma_unmap_single(ndev->dev.parent, lp->rx_bd_v[i].phys, - XTE_MAX_JUMBO_FRAME_SIZE, DMA_FROM_DEVICE); + ndev->mtu, DMA_FROM_DEVICE); dev_kfree_skb(lp->rx_skb[i]); } } @@ -274,7 +274,7 @@ static int temac_dma_bd_init(struct net_device *ndev) sizeof(*lp->rx_bd_v) * ((i + 1) % RX_BD_NUM); skb = netdev_alloc_skb_ip_align(ndev, - XTE_MAX_JUMBO_FRAME_SIZE); + ndev->mtu); if (skb == 0) { dev_err(&ndev->dev, "alloc_skb error %d\n", i); @@ -284,9 +284,9 @@ static int temac_dma_bd_init(struct net_device *ndev) /* returns physical address of skb->data */ lp->rx_bd_v[i].phys = dma_map_single(ndev->dev.parent, skb->data, -XTE_MAX_JUMBO_FRAME_SIZE, +ndev->mtu, DMA_FROM_DEVICE); - lp->rx_bd_v[i].len = XTE_MAX_JUMBO_FRAME_SIZE; + lp->rx_bd_v[i].len = ndev->mtu; lp->rx_bd_v[i].app0 = STS_CTRL_APP0_IRQONEND; } @@ -787,7 +787,7 @@ static void ll_temac_recv(struct net_device *ndev) ndev->stats.rx_bytes += length; new_skb = netdev_alloc_skb_ip_align(ndev, - XTE_MAX_JUMBO_FRAME_SIZE); + ndev->mtu); if (new_skb == 0) { dev_err(&ndev->dev, "no memory for new sk_buff\n"); @@ -797,9 +797,9 @@ static void ll_temac_recv(struct net_device *ndev) cur_p->app0 = STS_CTRL_APP0_IRQONEND; cur_p->phys = dma_map_single(ndev->dev.parent, new_skb->data, -XTE_MAX_JUMBO_FRAME_SIZE, +ndev->mtu, DMA_FROM_DEVICE); - cur_p->len = XTE_MAX_JUMBO_FRAME_SIZE; + cur_p->len = ndev->mtu; lp->rx_skb[lp->rx_bd_ci] = new_skb; lp->rx_bd_ci++; -- 1.7.0.4 -- 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/
[PATCH 06/11] net: ll_temac: Fix mdio initialization
Current driver required to have phy-node directly in the driver. After this fixed we can use standard structure. DTS fragment: phy-handle = <&phy0>; mdio { #address-cells = <1>; #size-cells = <0>; phy0: phy@7 { compatible = "marvell,88e"; device_type = "ethernet-phy"; reg = <7>; } ; } ; Signed-off-by: Michal Simek CC: Anirudha Sarangi CC: John Linn CC: Grant Likely CC: Rob Herring CC: David S. Miller --- drivers/net/ethernet/xilinx/ll_temac_main.c | 11 ++- drivers/net/ethernet/xilinx/ll_temac_mdio.c |5 +++-- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/drivers/net/ethernet/xilinx/ll_temac_main.c b/drivers/net/ethernet/xilinx/ll_temac_main.c index f8e3518..482b572 100644 --- a/drivers/net/ethernet/xilinx/ll_temac_main.c +++ b/drivers/net/ethernet/xilinx/ll_temac_main.c @@ -1108,14 +1108,15 @@ static int __devinit temac_of_probe(struct platform_device *op) } temac_set_mac_address(ndev, (void *)addr); - rc = temac_mdio_setup(lp, op->dev.of_node); - if (rc) - dev_warn(&op->dev, "error registering MDIO bus\n"); - lp->phy_node = of_parse_phandle(op->dev.of_node, "phy-handle", 0); - if (lp->phy_node) + if (lp->phy_node) { dev_dbg(lp->dev, "using PHY node %s (%p)\n", np->full_name, np); + rc = temac_mdio_setup(lp, op->dev.of_node); + if (rc) + dev_warn(&op->dev, "error registering MDIO bus\n"); + } + /* Add the device attributes */ rc = sysfs_create_group(&lp->dev->kobj, &temac_attr_group); if (rc) { diff --git a/drivers/net/ethernet/xilinx/ll_temac_mdio.c b/drivers/net/ethernet/xilinx/ll_temac_mdio.c index 8cf9d4f..634d898 100644 --- a/drivers/net/ethernet/xilinx/ll_temac_mdio.c +++ b/drivers/net/ethernet/xilinx/ll_temac_mdio.c @@ -63,6 +63,7 @@ int temac_mdio_setup(struct temac_local *lp, struct device_node *np) int clk_div; int rc, size; struct resource res; + struct device_node *np1 = of_get_parent(lp->phy_node); /* Calculate a reasonable divisor for the clock rate */ clk_div = 0x3f; /* worst-case default setting */ @@ -85,7 +86,7 @@ int temac_mdio_setup(struct temac_local *lp, struct device_node *np) if (!bus) return -ENOMEM; - of_address_to_resource(np, 0, &res); + of_address_to_resource(np1, 0, &res); snprintf(bus->id, MII_BUS_ID_SIZE, "%.8llx", (unsigned long long)res.start); bus->priv = lp; @@ -97,7 +98,7 @@ int temac_mdio_setup(struct temac_local *lp, struct device_node *np) lp->mii_bus = bus; - rc = of_mdiobus_register(bus, np); + rc = of_mdiobus_register(bus, np1); if (rc) goto err_register; -- 1.7.0.4 -- 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/
[PATCH 05/11] net: axienet: Enable VLAN support by default
The driver is using frame size for VLAN packets but does not enable VLAN IP option. Signed-off-by: Michal Simek CC: Anirudha Sarangi CC: John Linn CC: Grant Likely CC: Rob Herring CC: David S. Miller --- drivers/net/ethernet/xilinx/xilinx_axienet_main.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index 8d1db13..4ef148f 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -483,6 +483,7 @@ static void axienet_device_reset(struct net_device *ndev) __axienet_device_reset(lp, &ndev->dev, XAXIDMA_RX_CR_OFFSET); lp->max_frm_size = XAE_MAX_VLAN_FRAME_SIZE; + lp->options |= XAE_OPTION_VLAN; lp->options &= (~XAE_OPTION_JUMBO); if ((ndev->mtu > XAE_MTU) && -- 1.7.0.4 -- 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/
[PATCH 04/11] net: axienet: Additional MDIO clock functionality
The MDIO clock was previously hard coded and it is now calculated thanks to a patch from the community. Modify the Xilinx patch to get the clock frequency from the connected AXI bus instead of the CPU. Currently, the AXI ethernet mdio bus id is set as the mdio device node start address, but the mdio node don't have a start address. Use the start address of the AXI ethernet controller which connects to the MDIO bus instead. Signed-off-by: Wendy Liang Signed-off-by: Nico Augustijn CC: Anirudha Sarangi CC: John Linn CC: Grant Likely CC: Rob Herring CC: David S. Miller --- drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c | 76 +++-- 1 files changed, 40 insertions(+), 36 deletions(-) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c b/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c index e90e1f4..49acc1e 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c @@ -128,11 +128,11 @@ static int axienet_mdio_write(struct mii_bus *bus, int phy_id, int reg, int axienet_mdio_setup(struct axienet_local *lp, struct device_node *np) { int ret; - u32 clk_div, host_clock; - u32 *property_p; + u32 clk_div; struct mii_bus *bus; struct resource res; struct device_node *np1; + struct device_node *npp = 0; /* the ethernet controller device node */ /* clk_div can be calculated by deriving it from the equation: * fMDIO = fHOST / ((1 + clk_div) * 2) @@ -158,41 +158,46 @@ int axienet_mdio_setup(struct axienet_local *lp, struct device_node *np) * fHOST can be read from the flattened device tree as property * "clock-frequency" from the CPU */ - - np1 = of_find_node_by_name(NULL, "cpu"); - if (!np1) { - printk(KERN_WARNING "%s(): Could not find CPU device node.", - __func__); - printk(KERN_WARNING "Setting MDIO clock divisor to " - "default %d\n", DEFAULT_CLOCK_DIVISOR); - clk_div = DEFAULT_CLOCK_DIVISOR; - goto issue; - } - property_p = (u32 *) of_get_property(np1, "clock-frequency", NULL); - if (!property_p) { - printk(KERN_WARNING "%s(): Could not find CPU property: " - "clock-frequency.", __func__); - printk(KERN_WARNING "Setting MDIO clock divisor to " - "default %d\n", DEFAULT_CLOCK_DIVISOR); + np1 = of_get_parent(lp->phy_node); + if (np1) + npp = of_get_parent(np1); + if (!npp) { + dev_warn(lp->dev, + "Could not find ethernet controller device node."); + dev_warn(lp->dev, "Setting MDIO clock divisor to default %d\n", + DEFAULT_CLOCK_DIVISOR); clk_div = DEFAULT_CLOCK_DIVISOR; - goto issue; + } else { + u32 *property_p; + + property_p = (uint32_t *)of_get_property(npp, + "clock-frequency", NULL); + if (!property_p) { + dev_warn(lp->dev, "Could not find clock ethernet " \ + "controller property."); + dev_warn(lp->dev, +"Setting MDIO clock divisor to default %d\n", + DEFAULT_CLOCK_DIVISOR); + clk_div = DEFAULT_CLOCK_DIVISOR; + } else { + u32 host_clock = be32_to_cpup(property_p); + + clk_div = (host_clock / (MAX_MDIO_FREQ * 2)) - 1; + + /* If there is any remainder from the division of +* fHOST / (MAX_MDIO_FREQ * 2), then we need to add 1 +* to the clock divisor or we will surely be +* above 2.5 MHz */ + if (host_clock % (MAX_MDIO_FREQ * 2)) + clk_div++; + dev_dbg(lp->dev, "Setting MDIO clock divisor to %u " \ + "based on %u Hz host clock.\n", + clk_div, host_clock); + } } - host_clock = be32_to_cpup(property_p); - clk_div = (host_clock / (MAX_MDIO_FREQ * 2)) - 1; - /* If there is any remainder from the division of -* fHOST / (MAX_MDIO_FREQ * 2), then we need to add -* 1 to the clock divisor or we will surely be above 2.5 MHz */ - if (host_clock % (MAX_MDIO_FREQ * 2)) - clk_div++; - - printk(KERN_DEBUG "%s(): Setting MDIO clock divisor to %u based " - "on %u Hz host clock.\n", __func__, clk_div, host_clock); - - of_node_put(np1); -issue: - axienet_iow(lp, XAE_MDIO_MC_OFFSET, -
Re: [PATCH 08/11] net: ll_temac: Do not use fixed mtu size
On 10/04/2012 09:22 PM, Ben Hutchings wrote: On Thu, 2012-10-04 at 20:14 +0200, Michal Simek wrote: Use max mtu instead. [...] MTU does not include the Ethernet header so I have no idea how this is expected to work... Right. This is wrong fix. It should be the same as is in axienet. There is max_frm_size used which is mtu+header+tailer. I will fix it. Thanks, Michal -- Michal Simek, Ing. (M.Eng) w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/ Microblaze U-BOOT custodian -- 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 11/11] net: xilinx: Show csum in bootlog
On 10/04/2012 09:15 PM, Ben Hutchings wrote: On Thu, 2012-10-04 at 20:14 +0200, Michal Simek wrote: Just show current setting in bootlog. [...] --- a/drivers/net/ethernet/xilinx/ll_temac_main.c +++ b/drivers/net/ethernet/xilinx/ll_temac_main.c @@ -1052,12 +1052,14 @@ static int __devinit temac_of_probe(struct platform_device *op) /* Setup checksum offload, but default to off if not specified */ lp->temac_features = 0; p = (__be32 *)of_get_property(op->dev.of_node, "xlnx,txcsum", NULL); + dev_info(&op->dev, "TX_CSUM %d\n", be32_to_cpup(p)); if (p && be32_to_cpu(*p)) { lp->temac_features |= TEMAC_FEATURE_TX_CSUM; /* Can checksum TCP/UDP over IPv4. */ ndev->features |= NETIF_F_IP_CSUM; } p = (__be32 *)of_get_property(op->dev.of_node, "xlnx,rxcsum", NULL); + dev_info(&op->dev, "RX_CSUM %d\n", be32_to_cpup(p)); [...] Is there any particular reason you think this needs to be logged by default, rather than letting users run ethtool -k? I suggest using dev_dbg() instead. The reason was just to show it in the bootlog. I will check ethtool support for these drivers. Thanks for your comments, Michal -- Michal Simek, Ing. (M.Eng) w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/ Microblaze U-BOOT custodian -- 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 02/11] net: axienet: Add ioctl support
On 10/04/2012 09:17 PM, Ben Hutchings wrote: On Thu, 2012-10-04 at 20:14 +0200, Michal Simek wrote: Allow user to access the MDIO from userspace. Signed-off-by: Michal Simek CC: Anirudha Sarangi CC: John Linn CC: Grant Likely CC: Rob Herring CC: David S. Miller --- drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 15 +++ 1 files changed, 15 insertions(+), 0 deletions(-) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index 50167ab..a5b41cd 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -1053,6 +1053,20 @@ static void axienet_poll_controller(struct net_device *ndev) } #endif +/* Ioctl MII Interface */ +static int axienet_ioctl(struct net_device *dev, struct ifreq *rq, int cmd) +{ + struct axienet_local *priv = netdev_priv(dev); + + if (!netif_running(dev)) + return -EINVAL; Not sure this is the appropriate error code. + if (!priv->phy_dev) + return -ENODEV; Error code should be EOPNOTSUPP - the device is present but just doesn't support MDIO. ok. Thanks will fix it. Thanks, Michal -- Michal Simek, Ing. (M.Eng) w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/ Microblaze U-BOOT custodian -- 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 01/11] net: axienet: Remove NETIF_F_SG dropping for no checksum feature
Hi David, On 10/04/2012 08:26 PM, David Miller wrote: Sorry, no. I've announced on netdev very clearly that net-next submissions are not appropriate at this time and that only pure bug fixes should be submitted. Watch for the announcement on netdev of net-next openning up after the merge window closes, that's when you should resubmit this series. Sorry I should label it as RFC. Thanks, Michal -- Michal Simek, Ing. (M.Eng) w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/ Microblaze U-BOOT custodian -- 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 11/11] net: xilinx: Show csum in bootlog
On 10/04/2012 09:15 PM, Ben Hutchings wrote: On Thu, 2012-10-04 at 20:14 +0200, Michal Simek wrote: Just show current setting in bootlog. [...] --- a/drivers/net/ethernet/xilinx/ll_temac_main.c +++ b/drivers/net/ethernet/xilinx/ll_temac_main.c @@ -1052,12 +1052,14 @@ static int __devinit temac_of_probe(struct platform_device *op) /* Setup checksum offload, but default to off if not specified */ lp->temac_features = 0; p = (__be32 *)of_get_property(op->dev.of_node, "xlnx,txcsum", NULL); + dev_info(&op->dev, "TX_CSUM %d\n", be32_to_cpup(p)); if (p && be32_to_cpu(*p)) { lp->temac_features |= TEMAC_FEATURE_TX_CSUM; /* Can checksum TCP/UDP over IPv4. */ ndev->features |= NETIF_F_IP_CSUM; } p = (__be32 *)of_get_property(op->dev.of_node, "xlnx,rxcsum", NULL); + dev_info(&op->dev, "RX_CSUM %d\n", be32_to_cpup(p)); [...] Is there any particular reason you think this needs to be logged by default, rather than letting users run ethtool -k? I suggest using dev_dbg() instead. Ok. I have looked at it and there are missing some bits in ndev->features. Can you please check that my setting is correct? It is SG DMA ip/driver. ndev->features = NETIF_F_FRAGLIST | NETIF_F_SG With two options for csum on RX/TX. They can be selected independently. tx Partial csum over IPv4. -> NETIF_F_IP_CSUM tx Full csum. -> NETIF_F_HW_CSUM rx Full csum -> NETIF_F_RXCSUM Is there any option to support partial csum? Thanks, Michal -- Michal Simek, Ing. (M.Eng) w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/ Microblaze U-BOOT custodian -- 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] mtd: m25p80: Add support for Numonyx N25Q128
On 10/05/2012 09:50 AM, Jan Lübbe wrote: On Thu, 2012-10-04 at 19:40 +0200, Michal Simek wrote: From: Wendy Liang Add the id and sector mappings for SPI flash chips. An equivalent patch is already in David Woodhouse's l2-mtd tree: http://git.infradead.org/users/dedekind/l2-mtd.git/commitdiff/3105875f6b8902628caee2fd7821af43707c6bde Great thanks. Michal -- Michal Simek, Ing. (M.Eng) w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/ Microblaze U-BOOT custodian -- 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/
[GIT PULL] arch/microblaze changes for 3.7-rc1
Hi Linus, please pull the following changes. Thanks, Michal The following changes since commit a0d271cbfed1dd50278c6b06bead3d00ba0a88f9: Linus Torvalds (1): Linux 3.6 are available in the git repository at: git://git.monstr.eu/linux-2.6-microblaze.git next John Linn (1): microblaze: Added more support for PCI Michal Simek (10): microblaze: Add support for ioreadXX/iowriteXX_rep microblaze: Added fdt chosen capability for timer microblaze: Do not used hardcoded value in exception handler microblaze: Support 4k/16k/64k pages microblaze: Use predefined macro for ESR_DIZ microblaze: Remove additional andi which has been already done microblaze: Remove PAGE properties duplication microblaze: Fix bug with passing command line microblaze: Prefer to use pr_XXX instead of printk(KERN_XX) Revert "microblaze_mmu_v2: Update signal returning address" Paul Bolle (1): microblaze: clinkage.h Stephan Linz (1): microblaze: Improve failure handling for GPIO reset arch/microblaze/Kconfig |7 +-- arch/microblaze/include/asm/clinkage.h|1 - arch/microblaze/include/asm/io.h | 94 + arch/microblaze/include/asm/page.h|9 +-- arch/microblaze/include/asm/pci.h |2 + arch/microblaze/include/asm/pgtable.h |6 -- arch/microblaze/kernel/head.S | 14 +++-- arch/microblaze/kernel/hw_exception_handler.S | 61 ++--- arch/microblaze/kernel/reset.c| 21 +++--- arch/microblaze/kernel/setup.c| 15 ++-- arch/microblaze/kernel/signal.c |8 -- arch/microblaze/kernel/timer.c| 24 -- 12 files changed, 182 insertions(+), 80 deletions(-) delete mode 100644 arch/microblaze/include/asm/clinkage.h -- Michal Simek, Ing. (M.Eng) w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/ Microblaze U-BOOT custodian -- 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 RFT] microblaze: dma-mapping: support debug_dma_mapping_error
On 10/26/2012 01:29 AM, Shuah Khan wrote: Add support for debug_dma_mapping_error() call to avoid warning from debug_dma_unmap() interface when it checks for mapping error checked status. Without this patch, device driver failed to check map error warning is generated. Signed-off-by: Shuah Khan --- arch/microblaze/include/asm/dma-mapping.h |1 + 1 file changed, 1 insertion(+) diff --git a/arch/microblaze/include/asm/dma-mapping.h b/arch/microblaze/include/asm/dma-mapping.h index 01d2282..4451c7a 100644 --- a/arch/microblaze/include/asm/dma-mapping.h +++ b/arch/microblaze/include/asm/dma-mapping.h @@ -114,6 +114,7 @@ static inline void __dma_sync(unsigned long paddr, static inline int dma_mapping_error(struct device *dev, dma_addr_t dma_addr) { struct dma_map_ops *ops = get_dma_ops(dev); here should be empty line. + debug_dma_mapping_error(dev, dma_addr); if (ops->mapping_error) return ops->mapping_error(dev, dma_addr); Patch looks good to me. I was greping the latest mainline tree and I can't see reference to debug_dma_mapping_error. Can you tell me where this function is implemented. I have also seen that you have sent several similar patches like this. If you want to add it through any other tree here is my ACK: Acked-by: Michal Simek I will definitely keep my eyes on it. Thanks, Michal -- Michal Simek, Ing. (M.Eng) w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/ Microblaze U-BOOT custodian -- 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/
[GIT PULL] arm-soc: Xilinx zynq changes for v3.8
Hi Arnd and Olof, please add these patches to your arm-soc trees. All these patches are for xilinx zynq arm platform. You shouldn't have any problem to merge it there are no conficts with v3.7-rc4. Please let me know if you find out any problem. Thanks, Michal The following changes since commit 8f0d8163b50e01f398b14bcd4dc039ac5ab18d64: Linus Torvalds (1): Linux 3.7-rc3 are available in the git repository at: git://git.xilinx.com/linux-xlnx.git arm-next Josh Cartwright (4): zynq: use GIC device tree bindings zynq: use pl310 device tree bindings zynq: remove use of CLKDEV_LOOKUP zynq: move static peripheral mappings arch/arm/Kconfig |1 - arch/arm/Makefile |1 - arch/arm/boot/dts/zynq-ep107.dts | 19 +--- arch/arm/mach-zynq/common.c| 23 --- arch/arm/mach-zynq/include/mach/clkdev.h | 32 arch/arm/mach-zynq/include/mach/zynq_soc.h | 31 +-- 6 files changed, 40 insertions(+), 67 deletions(-) delete mode 100644 arch/arm/mach-zynq/include/mach/clkdev.h -- 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 1/8] ARM: zynq: move arm-specific sys_timer out of ttc
2012/10/31 Josh Cartwright : > Move the sys_timer definition out of ttc driver and make it part of the > common zynq code. This is preparation for renaming and COMMON_CLK > support. > > Signed-off-by: Josh Cartwright > --- > arch/arm/mach-zynq/common.c | 13 + > arch/arm/mach-zynq/common.h | 4 +--- > arch/arm/mach-zynq/timer.c | 10 +- > 3 files changed, 15 insertions(+), 12 deletions(-) > Tested-by: Michal Simek Looks good to me. I have added it to my testing branch and will provide path to mainline through xilinx arm-next branch Thanks, Michal -- 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 2/8] ARM: zynq: move ttc timer code to drivers/clocksource
2012/10/29 Josh Cartwright : > Suggested cleanup by Arnd Bergmann. Move the ttc timer.c code to > drivers/clocksource, and out of the mach-zynq directory. > > The common.h (which only held the timer declaration) was renamed to > xilinx_ttc.h and moved into include/linux. > > Signed-off-by: Josh Cartwright > Cc: Arnd Bergmann > --- > arch/arm/mach-zynq/Makefile| 2 +- > arch/arm/mach-zynq/common.c| 2 +- > drivers/clocksource/Makefile | 1 + > arch/arm/mach-zynq/timer.c => drivers/clocksource/xilinx_ttc.c | 1 - > arch/arm/mach-zynq/common.h => include/linux/xilinx_ttc.h | 4 ++-- > 5 files changed, 5 insertions(+), 5 deletions(-) > rename arch/arm/mach-zynq/timer.c => drivers/clocksource/xilinx_ttc.c (99%) > rename arch/arm/mach-zynq/common.h => include/linux/xilinx_ttc.h (91%) Not going to apply this patch till there is clean way how to move all drivers there. Especially I don't like to add xilinx_ttc.h to include/linux folder. Thanks, Michal -- 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 4/8] ARM: zynq: dts: split up device tree
2012/10/31 Josh Cartwright : > The purpose of the created zynq-7000.dtsi file is to describe the > hardware common to all Zynq 7000-based boards. Also, get rid of the > zynq-ep107 device tree, since it is not hardware anyone can purchase. > > Add a zc702 dts file based on the zynq-7000.dtsi. Add it to the > dts/Makefile so it is built with the 'dtbs' target. > > Signed-off-by: Josh Cartwright > --- > arch/arm/boot/dts/Makefile | 1 + > .../boot/dts/{zynq-ep107.dts => zynq-7000.dtsi}| 19 +++--- > arch/arm/boot/dts/zynq-zc702.dts | 30 > ++ > arch/arm/mach-zynq/common.c| 3 ++- > 4 files changed, 36 insertions(+), 17 deletions(-) > rename arch/arm/boot/dts/{zynq-ep107.dts => zynq-7000.dtsi} (79%) > create mode 100644 arch/arm/boot/dts/zynq-zc702.dts Not going to apply this. We need to finish our discussion in "[PATCH v4 1/5] zynq: use GIC device tree bindings" before. Definitely I like idea to use "xlnx,zynq-7000" generic model name. Thanks, Michal -- 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 3/8] ARM: zynq: dts: add description of the second uart
2012/10/31 Josh Cartwright : > The zynq-7000 has an additional UART at 0xE0001000. Describe it in the > device tree. > > Signed-off-by: Josh Cartwright > --- > arch/arm/boot/dts/zynq-ep107.dts | 7 +++ > 1 file changed, 7 insertions(+) Applied to my testing branch. Thanks, Michal -- 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 6/8] serial: xilinx_uartps: kill CONFIG_OF conditional
2012/10/30 Josh Cartwright : > The Zynq platform requires the use of CONFIG_OF. Remove the #ifdef > conditionals in the uartps driver. > > Signed-off-by: Josh Cartwright > --- > drivers/tty/serial/xilinx_uartps.c | 9 - > 1 file changed, 9 deletions(-) Please send this separately out of this patchset. Also if you do this change which is understandable, you should also add depends on OF Thanks, Michal -- 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 5/8] ARM: zynq: add COMMON_CLK support
2012/11/2 Josh Cartwright : > On Fri, Nov 02, 2012 at 04:12:21PM +0100, Lars-Peter Clausen wrote: >> On 11/02/2012 02:38 PM, Josh Cartwright wrote: >> > On Fri, Nov 02, 2012 at 10:33:44AM +0100, Lars-Peter Clausen wrote: >> >> On 10/31/2012 07:58 PM, Josh Cartwright wrote: > [...] >> >>> +static void __init zynq_periph_clk_setup(struct device_node *np) >> >>> +{ >> >>> + struct zynq_periph_clk *periph; >> >>> + const char *parent_names[3]; >> >>> + struct clk_init_data init; >> >>> + struct clk *clk; >> >>> + int err; >> >>> + u32 reg; >> >>> + int i; >> >>> + >> >>> + err = of_property_read_u32(np, "reg", ®); >> >>> + WARN_ON(err); >> >> >> >> Shouldn't the function abort if a error happens somewhere? Continuing here >> >> will lead to undefined behavior. Same is probably true for the other >> >> WARN_ONs. >> > >> > The way I see it is: the kernel is will be left in a bad state in the >> > case of any failure, regardless of if we bail out or continue. AFAICT, >> > there is no clean way to recover from a failure this early. >> > >> > Given that, it seems simpler (albeit marginally so) just to continue; so >> > that's what I chose to do. I'm not opposed to bailing out, just not >> > convinced it does anything for us. >> > >> The issue with this approach is that, while you get a warning, unexpected >> seemingly unrelated side-effects may happen later on. E.g. if no reg >> property for the clock is specified the reg variable will be uninitialized >> and contain whatever was on the stack before. The clock will be registered >> nonetheless and the boot process continues. Now if the clock is enabled a >> bit in a random register will be modified, which could result in strange and >> abnormal behavior, which can be very hard to track down. > > Okay.but any reasonable person would start their debugging quest at > the source of the WARN_ON. If someone sees the WARN_ON message but > stupidly chooses to ignore it, they deserves to spend the time trying to > track down abnormal behavior, so I'm still not convinced. I am with Lars. You would be surprised how many people do no read bootlog. It should be handled better. Thanks, Michal -- 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 8/8] clocksource: xilinx_ttc: add OF_CLK support
2012/10/31 Josh Cartwright : > Add support for retrieving TTC configuration from device tree. This > includes the ability to pull information about the driving clocks from > the of_clk bindings. > > Signed-off-by: Josh Cartwright > --- > arch/arm/boot/dts/zynq-7000.dtsi | 53 > arch/arm/boot/dts/zynq-zc702.dts | 10 ++ > drivers/clocksource/xilinx_ttc.c | 273 > ++- > 3 files changed, 218 insertions(+), 118 deletions(-) > > diff --git a/arch/arm/boot/dts/zynq-7000.dtsi > b/arch/arm/boot/dts/zynq-7000.dtsi > index 5fb763f..9a2442c 100644 > --- a/arch/arm/boot/dts/zynq-7000.dtsi > +++ b/arch/arm/boot/dts/zynq-7000.dtsi > @@ -109,5 +109,58 @@ > }; > }; > }; > + > + ttc0: ttc0@f8001000 { > + #address-cells = <1>; > + #size-cells = <0>; > + compatible = "xlnx,ttc"; > + reg = <0xF8001000 0x1000>; > + clocks = <&cpu_clk 3>; > + clock-names = "cpu_1x"; > + clock-ranges; > + > + ttc0_0: ttc0.0 { > + status = "disabled"; > + reg = <0>; > + interrupts = <0 10 4>; > + }; > + ttc0_1: ttc0.1 { > + status = "disabled"; > + reg = <1>; > + interrupts = <0 11 4>; > + }; > + ttc0_2: ttc0.2 { > + status = "disabled"; > + reg = <2>; > + interrupts = <0 12 4>; > + }; > + }; > + > + ttc1: ttc0@f8002000 { Also this is ttc1: ttc1. These type of faults can be simple removed by proper dts node generation. Thanks, Michal -- Michal Simek, Ing. (M.Eng) w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/ Microblaze U-BOOT custodian -- 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 RFT] microblaze: dma-mapping: support debug_dma_mapping_error
On 10/26/2012 04:58 PM, Shuah Khan wrote: On Fri, 2012-10-26 at 10:29 +0200, Michal Simek wrote: On 10/26/2012 01:29 AM, Shuah Khan wrote: Add support for debug_dma_mapping_error() call to avoid warning from debug_dma_unmap() interface when it checks for mapping error checked status. Without this patch, device driver failed to check map error warning is generated. Signed-off-by: Shuah Khan --- arch/microblaze/include/asm/dma-mapping.h |1 + 1 file changed, 1 insertion(+) diff --git a/arch/microblaze/include/asm/dma-mapping.h b/arch/microblaze/include/asm/dma-mapping.h index 01d2282..4451c7a 100644 --- a/arch/microblaze/include/asm/dma-mapping.h +++ b/arch/microblaze/include/asm/dma-mapping.h @@ -114,6 +114,7 @@ static inline void __dma_sync(unsigned long paddr, static inline int dma_mapping_error(struct device *dev, dma_addr_t dma_addr) { struct dma_map_ops *ops = get_dma_ops(dev); here should be empty line. + debug_dma_mapping_error(dev, dma_addr); if (ops->mapping_error) return ops->mapping_error(dev, dma_addr); Patch looks good to me. I was greping the latest mainline tree and I can't see reference to debug_dma_mapping_error. Can you tell me where this function is implemented. I have also seen that you have sent several similar patches like this. If you want to add it through any other tree here is my ACK: Acked-by: Michal Simek I will definitely keep my eyes on it. debug_dma_mapping_error() patch is in linux-next. I probably should send all of these patches marked explicitly for linux-next. Ok. It means that this patch should be added to the tree which contains this patch. You can find it out through linux-next. What it is interesting to me is that arm tree has it there but implementation is missing in Linus tree. [linux-2.6.x]$ grep -rn "debug_dma_mapping_erro" arch/ arch/arm/include/asm/dma-mapping.h:94: debug_dma_mapping_error(dev, dma_addr); Thanks, Michal -- Michal Simek, Ing. (M.Eng) w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/ Microblaze U-BOOT custodian -- 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 v4 1/5] zynq: use GIC device tree bindings
Hi Josh, > -Original Message- > From: Josh Cartwright [mailto:josh.cartwri...@ni.com] > Sent: Wednesday, October 24, 2012 10:03 PM > To: a...@kernel.org; Arnd Bergmann > Cc: linux-kernel@vger.kernel.org; linux-arm-ker...@lists.infradead.org; John > Linn; Nick Bowler; Michal Simek > Subject: [PATCH v4 1/5] zynq: use GIC device tree bindings > > The Zynq uses the cortex-a9-gic. This eliminates the need to hardcode > register > addresses. > > Signed-off-by: Josh Cartwright > Cc: John Linn > Acked-by: Arnd Bergmann > --- > arch/arm/boot/dts/zynq-ep107.dts | 8 +--- > arch/arm/mach-zynq/common.c| 7 ++- > arch/arm/mach-zynq/include/mach/zynq_soc.h | 2 -- > 3 files changed, 11 insertions(+), 6 deletions(-) > > diff --git a/arch/arm/boot/dts/zynq-ep107.dts b/arch/arm/boot/dts/zynq- > ep107.dts > index 37ca192..7bfff4a 100644 > --- a/arch/arm/boot/dts/zynq-ep107.dts > +++ b/arch/arm/boot/dts/zynq-ep107.dts > @@ -36,10 +36,12 @@ > ranges; > > intc: interrupt-controller@f8f01000 { > + compatible = "arm,cortex-a9-gic"; > + #interrupt-cells = <3>; If you change git to 3 cells format you should also change it for uart below. Also will be the best to remove this dts entirely and replace it by existing Platform. Thanks, Michal This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately. -- 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 v4 2/5] zynq: use pl310 device tree bindings
> -Original Message- > From: Josh Cartwright [mailto:josh.cartwri...@ni.com] > Sent: Wednesday, October 24, 2012 10:04 PM > To: a...@kernel.org; Arnd Bergmann > Cc: linux-kernel@vger.kernel.org; linux-arm-ker...@lists.infradead.org; John > Linn; Nick Bowler; Michal Simek > Subject: [PATCH v4 2/5] zynq: use pl310 device tree bindings > > The Zynq has a PL310 L2 cache controller. Convert in-tree uses to using the > device tree. > > Signed-off-by: Josh Cartwright > Cc: John Linn > Acked-by: Arnd Bergmann > --- > arch/arm/boot/dts/zynq-ep107.dts | 9 + > arch/arm/mach-zynq/common.c| 9 + > arch/arm/mach-zynq/include/mach/zynq_soc.h | 4 > 3 files changed, 10 insertions(+), 12 deletions(-) > > diff --git a/arch/arm/boot/dts/zynq-ep107.dts b/arch/arm/boot/dts/zynq- > ep107.dts > index 7bfff4a..87204d7 100644 > --- a/arch/arm/boot/dts/zynq-ep107.dts > +++ b/arch/arm/boot/dts/zynq-ep107.dts > @@ -44,6 +44,15 @@ > <0xF8F00100 0x100>; > }; > > + L2: cache-controller { > + compatible = "arm,pl310-cache"; > + reg = <0xF8F02000 0x1000>; > + arm,data-latency = <2 3 2>; > + arm,tag-latency = <2 3 2>; > + cache-unified; > + cache-level = <2>; > + }; > + > uart0: uart@e000 { > compatible = "xlnx,xuartps"; > reg = <0xE000 0x1000>; > diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c > index d73963b..056091a 100644 > --- a/arch/arm/mach-zynq/common.c > +++ b/arch/arm/mach-zynq/common.c > @@ -45,12 +45,10 @@ static struct of_device_id zynq_of_bus_ids[] __initdata = > { > */ > static void __init xilinx_init_machine(void) { -#ifdef CONFIG_CACHE_L2X0 > /* >* 64KB way size, 8-way associativity, parity disabled >*/ > - l2x0_init(PL310_L2CC_BASE, 0x0206, 0xF0F0); > -#endif > + l2x0_of_init(0x0206, 0xF0F0); > > of_platform_bus_probe(NULL, zynq_of_bus_ids, NULL); } @@ -83,11 > +81,6 @@ static struct map_desc io_desc[] __initdata = { > .pfn= __phys_to_pfn(SCU_PERIPH_PHYS), > .length = SZ_8K, > .type = MT_DEVICE, > - }, { > - .virtual= PL310_L2CC_VIRT, > - .pfn= __phys_to_pfn(PL310_L2CC_PHYS), > - .length = SZ_4K, > - .type = MT_DEVICE, > }, > > #ifdef CONFIG_DEBUG_LL > diff --git a/arch/arm/mach-zynq/include/mach/zynq_soc.h b/arch/arm/mach- > zynq/include/mach/zynq_soc.h > index 3d1c6a6..218283a 100644 > --- a/arch/arm/mach-zynq/include/mach/zynq_soc.h > +++ b/arch/arm/mach-zynq/include/mach/zynq_soc.h > @@ -25,9 +25,6 @@ > #define TTC0_PHYS0xF8001000 > #define TTC0_VIRTTTC0_PHYS > > -#define PL310_L2CC_PHYS 0xF8F02000 > -#define PL310_L2CC_VIRT PL310_L2CC_PHYS > - > #define SCU_PERIPH_PHYS 0xF8F0 > #define SCU_PERIPH_VIRT SCU_PERIPH_PHYS > > @@ -35,7 +32,6 @@ > > #define TTC0_BASE IOMEM(TTC0_VIRT) > #define SCU_PERIPH_BASE IOMEM(SCU_PERIPH_VIRT) > -#define PL310_L2CC_BASE IOMEM(PL310_L2CC_VIRT) > > /* > * Mandatory for CONFIG_LL_DEBUG, UART is mapped virtual = physical > -- > 1.8.0 > This is ok. No changes are necessary. Acked-by: Michal Simek Please add my acked-by line to this patch to the v5 series. When all patches are ready I will apply it to zynq next branch at git.xilnx.com. Thanks, Michal This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately. -- 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 v4 4/5] ARM: annotate VMALLOC_END definition with _AC
> -Original Message- > From: Josh Cartwright [mailto:josh.cartwri...@ni.com] > Sent: Wednesday, October 24, 2012 10:05 PM > To: a...@kernel.org; Arnd Bergmann > Cc: linux-kernel@vger.kernel.org; linux-arm-ker...@lists.infradead.org; John > Linn; Nick Bowler; Michal Simek > Subject: [PATCH v4 4/5] ARM: annotate VMALLOC_END definition with _AC > > This makes the definition of VMALLOC_END suitable for use within assembly > code. This is necessary to allow the use of VMALLOC_END in defining where the > early uart is mapped for use with DEBUG_LL. > > Signed-off-by: Josh Cartwright > Acked-by: Arnd Bergmann > --- > arch/arm/include/asm/pgtable.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h > index 08c1231..72904a2 100644 > --- a/arch/arm/include/asm/pgtable.h > +++ b/arch/arm/include/asm/pgtable.h > @@ -40,7 +40,7 @@ > */ > #define VMALLOC_OFFSET (8*1024*1024) > #define VMALLOC_START(((unsigned long)high_memory + > VMALLOC_OFFSET) & ~(VMALLOC_OFFSET-1)) > -#define VMALLOC_END 0xff00UL > +#define VMALLOC_END _AC(0xff00,UL) This shouldn't be the part of this series but should go to mainline through different tree. Arnd, Olof: Can you take this patch to your arm-soc tree? I don't think it is a good workstyle to propose it to mainline through zynq soc tree. What do you think? Thanks, Michal -- 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 v4 1/5] zynq: use GIC device tree bindings
Hi Josh > -Original Message- > From: Josh Cartwright [mailto:josh.cartwri...@ni.com] > Sent: Saturday, October 27, 2012 4:01 PM > To: Michal Simek > Cc: a...@kernel.org; Arnd Bergmann; linux-kernel@vger.kernel.org; linux-arm- > ker...@lists.infradead.org; John Linn; Nick Bowler > Subject: Re: [PATCH v4 1/5] zynq: use GIC device tree bindings > > On Sat, Oct 27, 2012 at 01:39:00PM +, Michal Simek wrote: > > Hi Josh, > > > > > -Original Message- > > > From: Josh Cartwright [mailto:josh.cartwri...@ni.com] > > > Sent: Wednesday, October 24, 2012 10:03 PM > > > To: a...@kernel.org; Arnd Bergmann > > > Cc: linux-kernel@vger.kernel.org; > > > linux-arm-ker...@lists.infradead.org; John Linn; Nick Bowler; Michal > > > Simek > > > Subject: [PATCH v4 1/5] zynq: use GIC device tree bindings > > > > > > The Zynq uses the cortex-a9-gic. This eliminates the need to > > > hardcode register addresses. > > > > > > Signed-off-by: Josh Cartwright > > > Cc: John Linn > > > Acked-by: Arnd Bergmann > > > --- > > > arch/arm/boot/dts/zynq-ep107.dts | 8 +--- > > > arch/arm/mach-zynq/common.c| 7 ++- > > > arch/arm/mach-zynq/include/mach/zynq_soc.h | 2 -- > > > 3 files changed, 11 insertions(+), 6 deletions(-) > > > > > > diff --git a/arch/arm/boot/dts/zynq-ep107.dts > > > b/arch/arm/boot/dts/zynq- ep107.dts index 37ca192..7bfff4a 100644 > > > --- a/arch/arm/boot/dts/zynq-ep107.dts > > > +++ b/arch/arm/boot/dts/zynq-ep107.dts > > > @@ -36,10 +36,12 @@ > > > ranges; > > > > > > intc: interrupt-controller@f8f01000 { > > > + compatible = "arm,cortex-a9-gic"; > > > + #interrupt-cells = <3>; > > > > If you change git to 3 cells format you should also change it for uart > > below. > > > > Also will be the best to remove this dts entirely and replace it by > > existing Platform. > > Do you mean to say get rid of the ep107 entirely and replace it with, for > example, a zc702 dts? Yes, Ep107 platform is nothing what you can buy, that's why it is not the platform Which should be supported in mainline. Supporting zc702 make sense. > > I have a follow up patchset which splits off a zynq-7000.dtsi and a zynq- > zc702.dts, and which also fixes the 3 interrupt cell problem. I am not big fan to use dtsi solution because dts can be simple generated directly >From Xilinx design tool based on your hw design. That's why I can't see any >benefit To have dtsi file. > Would you like for me to pull this into v5, or spin up a separate patch > series? Definitely not. I have checked patches but haven't got it work on the zc702. Not sure if you have run it on real hw or just on the qemu as you have mentioned In 5/5. Thanks, Michal -- 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 v4 1/5] zynq: use GIC device tree bindings
> -Original Message- > From: Josh Cartwright [mailto:josh.cartwri...@ni.com] > Sent: Saturday, October 27, 2012 4:43 PM > To: Michal Simek > Cc: a...@kernel.org; Arnd Bergmann; linux-kernel@vger.kernel.org; linux-arm- > ker...@lists.infradead.org; John Linn; Nick Bowler > Subject: Re: [PATCH v4 1/5] zynq: use GIC device tree bindings > > On Sat, Oct 27, 2012 at 02:06:45PM +, Michal Simek wrote: > [...] > > I am not big fan to use dtsi solution because dts can be simple > > generated directly From Xilinx design tool based on your hw design. > > That's why I can't see any benefit To have dtsi file. > > Can I ask you to reconsider? I am open to all solution which will help others. I am not definitely saying NO for this features I just haven't found a reason to support it. > We, for example, don't make any use of the Xilinx > dev tools to generate our device trees. Ok. How does your working flow looks like? I mean you don't get any information from hardware guys how does your hw design look like? > Having a dtsi allows for easy extension > of the zynq-7000 platform for our boards, without having to carry duplicate > data. ok. I think that make sense if you send the next your series as RFC to see how exactly you would like to use it. In my workflow we generate DTS directly from design tool which I expect your hw guys use because it is probably needed to generate boot.bin/fsbl/etc. Then there is one more additional step to setup device-tree bsp to generate DTS which directly fits to your HW design. If you have the same boards with different programmable logic I understand that you would like to share that PS part and then just add it that IPs in PL. > Is it going to be expected that users building kernels for their zynq targets > have > access to the Xilinx EDK? Definitely not. You can do it just once and of course you can write it by hand and then just reusing. >From my point of view. You have to use design tools at least once to get >bitstream and boot.bin with fsbl. Please correct me if I am wrong. In this step you can use device-tree BSP to generate DTS ( I doesn't need to be perfect with all attached devices on i2c,spi, phys, etc but it reflects your hardware). You will get it in some seconds and your dts has correct irq numbers/ip description, compatible strings, addresses, position in the system - if you use bus bridges, etc) and you can just directly use it and kernel will boot. If you need to do changes in dts by hand, you can of course do it. > > > Would you like for me to pull this into v5, or spin up a separate patch > > > series? > > > > Definitely not. I have checked patches but haven't got it work on the zc702. > > Not sure if you have run it on real hw or just on the qemu as you have > > mentioned In 5/5. > > You're likely running into the issue Nick has identified in the thread for > patch 5 > where the chosen virtual address for the uart doesn't seem to work: > http://www.spinics.net/lists/arm-kernel/msg203141.html > > We haven't yet identified the root cause; any insight you can provide here > would be beneficial. > > Otherwise, I'm considering reworking patch 5 to move the uart mapping to a > known working location. I just need to get some time to catch you. thanks, Michal -- 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 v4 3/5] zynq: remove use of CLKDEV_LOOKUP
> -Original Message- > From: Josh Cartwright [mailto:josh.cartwri...@ni.com] > Sent: Wednesday, October 24, 2012 10:04 PM > To: a...@kernel.org; Arnd Bergmann > Cc: linux-kernel@vger.kernel.org; linux-arm-ker...@lists.infradead.org; John > Linn; Nick Bowler; Michal Simek > Subject: [PATCH v4 3/5] zynq: remove use of CLKDEV_LOOKUP > > The Zynq support in mainline does not (yet) make use of any of the generic clk > or clk lookup functionality. Remove what is upstream for now, until the > out-of- > tree implementation is in suitable form for merging. > > An important side effect of this patch is that it allows the building of a > Zynq > kernel without running into unresolved symbol problems: > >drivers/built-in.o: In function `amba_get_enable_pclk': >clkdev.c:(.text+0x444): undefined reference to `clk_enable' >drivers/built-in.o: In function `amba_remove': >clkdev.c:(.text+0x488): undefined reference to `clk_disable' >drivers/built-in.o: In function `amba_probe': >clkdev.c:(.text+0x540): undefined reference to `clk_disable' >drivers/built-in.o: In function `amba_device_add': >clkdev.c:(.text+0x77c): undefined reference to `clk_disable' >drivers/built-in.o: In function `enable_clock': >clkdev.c:(.text+0x29738): undefined reference to `clk_enable' >drivers/built-in.o: In function `disable_clock': >clkdev.c:(.text+0x29778): undefined reference to `clk_disable' >drivers/built-in.o: In function `__pm_clk_remove': >clkdev.c:(.text+0x297f8): undefined reference to `clk_disable' >drivers/built-in.o: In function `pm_clk_suspend': >clkdev.c:(.text+0x29bc8): undefined reference to `clk_disable' >drivers/built-in.o: In function `pm_clk_resume': >clkdev.c:(.text+0x29c28): undefined reference to `clk_enable' >make[2]: *** [vmlinux] Error 1 >make[1]: *** [sub-make] Error 2 >make: *** [all] Error 2 > > In addition, eliminate Zynq's "use" of the versatile platform, as it is no > longer > needed. As Nick Bowler points out: > >For the record, I think this was introduced by commit 56a34b03ff427 >("ARM: versatile: Make plat-versatile clock optional") which forgot to >select PLAT_VERSATILE_CLOCK on Zynq. This is not all that surprising, >because the fact that Zynq "uses" PLAT_VERSATILE is secretly hidden in >the Makefile. > >Nevertheless, the only feature from versatile that Zynq needed was the >clock support, so this patch should *also* delete the secret use of >plat-versatile by removing this line from arch/arm/Makefile: > > plat-$(CONFIG_ARCH_ZYNQ) += versatile > > Signed-off-by: Josh Cartwright > Cc: John Linn > Acked-by: Arnd Bergmann > --- > arch/arm/Kconfig | 1 - > arch/arm/Makefile| 1 - > arch/arm/mach-zynq/common.c | 1 - > arch/arm/mach-zynq/include/mach/clkdev.h | 32 > > 4 files changed, 35 deletions(-) > delete mode 100644 arch/arm/mach-zynq/include/mach/clkdev.h > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index cce4f8d..de70d99 100644 > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -946,7 +946,6 @@ config ARCH_ZYNQ > bool "Xilinx Zynq ARM Cortex A9 Platform" > select ARM_AMBA > select ARM_GIC > - select CLKDEV_LOOKUP > select CPU_V7 > select GENERIC_CLOCKEVENTS > select ICST > diff --git a/arch/arm/Makefile b/arch/arm/Makefile index 451757d..8dbab2d > 100644 > --- a/arch/arm/Makefile > +++ b/arch/arm/Makefile > @@ -197,7 +197,6 @@ machine-$(CONFIG_ARCH_ZYNQ) += zynq > # by CONFIG_* macro name. > plat-$(CONFIG_ARCH_OMAP) += omap > plat-$(CONFIG_ARCH_S3C64XX) += samsung > -plat-$(CONFIG_ARCH_ZYNQ) += versatile > plat-$(CONFIG_PLAT_IOP) += iop > plat-$(CONFIG_PLAT_NOMADIK) += nomadik > plat-$(CONFIG_PLAT_ORION)+= orion > diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c > index 056091a..ba48f06 100644 > --- a/arch/arm/mach-zynq/common.c > +++ b/arch/arm/mach-zynq/common.c > @@ -31,7 +31,6 @@ > #include > > #include > -#include > #include "common.h" > > static struct of_device_id zynq_of_bus_ids[] __initdata = { diff --git > a/arch/arm/mach-zynq/include/mach/clkdev.h b/arch/arm/mach- > zynq/include/mach/clkdev.h > deleted file mode 100644 > index c6e73d8..000 > --- a/arch/arm/mach-zynq/include/mach/clkdev.h > +++ /dev/null > @@ -1,32 +0,0 @@ > -/* > - * arch/arm/mac
RE: [PATCH v4 5/5] zynq: move static peripheral mappings
HI Josh and Nick, look below. > -Original Message- > From: Josh Cartwright [mailto:josh.cartwri...@ni.com] > Sent: Friday, October 26, 2012 3:03 AM > To: Nick Bowler > Cc: a...@kernel.org; Arnd Bergmann; linux-kernel@vger.kernel.org; linux-arm- > ker...@lists.infradead.org; John Linn; Michal Simek > Subject: Re: [PATCH v4 5/5] zynq: move static peripheral mappings > > On Thu, Oct 25, 2012 at 06:41:08PM -0400, Nick Bowler wrote: > > On 2012-10-25 16:29 -0500, Josh Cartwright wrote: > > > On Thu, Oct 25, 2012 at 04:17:01PM -0400, Nick Bowler wrote: > > > > Did you test this on any real hardware? I can't get the ZC702 to > > > > work with the UART mapped at this address (this ends up being > > > > mapped at 0xFEFFF000), although I can't for the life of me figure > > > > out why the virtual address even matters. Note that for the > > > > ZC702, the physical address of the "main" UART is 0xE0001000. > > Good news is you're not crazy; I was able to duplicate the problem here. > > > If I were to guess, I would guess that, except for when it "Works", > > the really really early printk stuff isn't actually hitting the uart > > at all. The "Fails" case would then be due to the stray writes > > crashing the board, and the "Truncated" case due to the stray writes > > being (ostensibly) benign. > > If I'm not mistaken, this hypothesis is predicated on the early bootup code > establishing a (linear?) mapping for addresses > VMALLOC_START; before the > mdesc->map_io() is even handled. That seems odd to me. > > > But I really have no way right now to test this hypothesis, since I > > can't print anything in the failing case. > > Not sure if I'll be able to get anything meaningful out of it yet (I've not > historically had good luck with Xilinx's debugging tools), but I did finally > get a > JTAG debugger hooked up to the zc702. I'll see if I can get any useful > information tomorrow. I have seen the same problem on zc702. I will debug it. Josh: the best will be if you can send v5 for patches 1-3 (1 with small changes in dts - uart) which I will apply to arm-next. 4/5 should go out of zynq subtree, it means directly to arm-soc or via Russel's tree. 5/5 + Nick patch should be tested. Thanks, Michal -- 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 v4 0/5] zynq subarch cleanups
Hi Josh, Michal- > > Here is a v5 of the zynq cleanup patchset that addresses your feedback. I've > intentionally left patches 4 and 5 in the set until we figure out the > appropriate > way to get them in tree (feel free to just apply 1-3) I am ok to pick just several patches from your patchset. But this is no definitely good working style. Not expert for submission process but I think that if there is one broken patch maintainer shouldn't apply it. Can someone else check this? > I've also moved the uart mapping in patch 5 to a known working address, until > we can work out what is happening there. This should allow this patchset to > be > applied and have the zc702 boot. Will look at it and apply if works. > You had suggested removing/renaming the zynq-ep107.dts; it wasn't clear > whether you had wanted that in this patchset or not. I'm going to assume not. > I'll follow up with this, after this patchset is applied, if that works for > you. Not in this patchset. Sure feel free to send it. Thanks, Michal -- 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 v4 1/5] zynq: use GIC device tree bindings
> -Original Message- > From: Josh Cartwright [mailto:josh.cartwri...@ni.com] > Sent: Thursday, October 18, 2012 2:47 AM > To: a...@kernel.org; Arnd Bergmann > Cc: linux-kernel@vger.kernel.org; linux-arm-ker...@lists.infradead.org; John > Linn; Nick Bowler; Michal Simek > Subject: [PATCH v4 1/5] zynq: use GIC device tree bindings > > The Zynq uses the cortex-a9-gic. This eliminates the need to hardcode > register > addresses. > > Signed-off-by: Josh Cartwright > Cc: John Linn > Acked-by: Arnd Bergmann > --- > arch/arm/boot/dts/zynq-ep107.dts | 10 ++ > arch/arm/mach-zynq/common.c| 7 ++- > arch/arm/mach-zynq/include/mach/zynq_soc.h | 2 -- > 3 files changed, 12 insertions(+), 7 deletions(-) > Applied to http://git.xilinx.com/?p=linux-xlnx.git;a=shortlog;h=refs/heads/arm-next Thanks, Michal -- 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 v4 2/5] zynq: use pl310 device tree bindings
> -Original Message- > From: Josh Cartwright [mailto:josh.cartwri...@ni.com] > Sent: Wednesday, October 24, 2012 12:34 AM > To: a...@kernel.org; Arnd Bergmann > Cc: linux-kernel@vger.kernel.org; linux-arm-ker...@lists.infradead.org; John > Linn; Nick Bowler; Michal Simek > Subject: [PATCH v4 2/5] zynq: use pl310 device tree bindings > > The Zynq has a PL310 L2 cache controller. Convert in-tree uses to using the > device tree. > > Signed-off-by: Josh Cartwright > Cc: John Linn > Acked-by: Arnd Bergmann > Acked-by: Michal Simek > --- > arch/arm/boot/dts/zynq-ep107.dts | 9 + > arch/arm/mach-zynq/common.c| 9 + > arch/arm/mach-zynq/include/mach/zynq_soc.h | 4 > 3 files changed, 10 insertions(+), 12 deletions(-) Applied to http://git.xilinx.com/?p=linux-xlnx.git;a=shortlog;h=refs/heads/arm-next Thanks, Michal -- 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 v4 3/5] zynq: remove use of CLKDEV_LOOKUP
> -Original Message- > From: Josh Cartwright [mailto:josh.cartwri...@ni.com] > Sent: Sunday, October 21, 2012 6:17 PM > To: a...@kernel.org; Arnd Bergmann > Cc: linux-kernel@vger.kernel.org; linux-arm-ker...@lists.infradead.org; John > Linn; Nick Bowler; Michal Simek > Subject: [PATCH v4 3/5] zynq: remove use of CLKDEV_LOOKUP > > The Zynq support in mainline does not (yet) make use of any of the generic clk > or clk lookup functionality. Remove what is upstream for now, until the > out-of- > tree implementation is in suitable form for merging. > > An important side effect of this patch is that it allows the building of a > Zynq > kernel without running into unresolved symbol problems: > >drivers/built-in.o: In function `amba_get_enable_pclk': >clkdev.c:(.text+0x444): undefined reference to `clk_enable' >drivers/built-in.o: In function `amba_remove': >clkdev.c:(.text+0x488): undefined reference to `clk_disable' >drivers/built-in.o: In function `amba_probe': >clkdev.c:(.text+0x540): undefined reference to `clk_disable' >drivers/built-in.o: In function `amba_device_add': >clkdev.c:(.text+0x77c): undefined reference to `clk_disable' >drivers/built-in.o: In function `enable_clock': >clkdev.c:(.text+0x29738): undefined reference to `clk_enable' >drivers/built-in.o: In function `disable_clock': >clkdev.c:(.text+0x29778): undefined reference to `clk_disable' >drivers/built-in.o: In function `__pm_clk_remove': >clkdev.c:(.text+0x297f8): undefined reference to `clk_disable' >drivers/built-in.o: In function `pm_clk_suspend': >clkdev.c:(.text+0x29bc8): undefined reference to `clk_disable' >drivers/built-in.o: In function `pm_clk_resume': >clkdev.c:(.text+0x29c28): undefined reference to `clk_enable' >make[2]: *** [vmlinux] Error 1 >make[1]: *** [sub-make] Error 2 >make: *** [all] Error 2 > > In addition, eliminate Zynq's "use" of the versatile platform, as it is no > longer > needed. As Nick Bowler points out: > > For the record, I think this was introduced by commit 56a34b03ff427 > ("ARM: versatile: Make plat-versatile clock optional") which forgot to > select PLAT_VERSATILE_CLOCK on Zynq. This is not all that surprising, > because the fact that Zynq "uses" PLAT_VERSATILE is secretly hidden in > the Makefile. > > Nevertheless, the only feature from versatile that Zynq needed was the > clock support, so this patch should *also* delete the secret use of > plat-versatile by removing this line from arch/arm/Makefile: > > plat-$(CONFIG_ARCH_ZYNQ) += versatile > > Signed-off-by: Josh Cartwright > Cc: John Linn > Acked-by: Arnd Bergmann > Tested-by: Michal Simek > --- > arch/arm/Kconfig | 1 - > arch/arm/Makefile| 1 - > arch/arm/mach-zynq/common.c | 1 - > arch/arm/mach-zynq/include/mach/clkdev.h | 32 > > 4 files changed, 35 deletions(-) > delete mode 100644 arch/arm/mach-zynq/include/mach/clkdev.h > Applied to http://git.xilinx.com/?p=linux-xlnx.git;a=shortlog;h=refs/heads/arm-next Thanks, Michal -- 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 v4 5/5] zynq: move static peripheral mappings
> -Original Message- > From: Josh Cartwright [mailto:josh.cartwri...@ni.com] > Sent: Monday, October 22, 2012 4:16 AM > To: a...@kernel.org; Arnd Bergmann > Cc: linux-kernel@vger.kernel.org; linux-arm-ker...@lists.infradead.org; John > Linn; Nick Bowler; Michal Simek > Subject: [PATCH v4 5/5] zynq: move static peripheral mappings > > Shifting them up into the vmalloc region prevents the following warning, when > booting a zynq qemu target with more than 512mb of RAM: > > BUG: mapping for 0xe000 at 0xe000 out of vmalloc space > > In addition, it allows for reuse of these mappings when the proper drivers > issue > requests via ioremap(). > > There are currently unknown issues with the early uart mapping. For now, the > uart will be mapped to a known working address. > > Signed-off-by: Josh Cartwright > Cc: John Linn > Acked-by: Arnd Bergmann > --- > arch/arm/mach-zynq/common.c| 6 +++--- > arch/arm/mach-zynq/include/mach/zynq_soc.h | 25 +++-- > 2 files changed, 18 insertions(+), 13 deletions(-) > Tested and applied to http://git.xilinx.com/?p=linux-xlnx.git;a=shortlog;h=refs/heads/arm-next Thanks, Michal This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately. -- 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 v4 0/5] zynq subarch cleanups
> -Original Message- > From: Josh Cartwright [mailto:josh.cartwri...@ni.com] > Sent: Monday, October 29, 2012 2:36 PM > To: Michal Simek > Cc: a...@kernel.org; linux-kernel@vger.kernel.org; linux-arm- > ker...@lists.infradead.org; John Linn; Nick Bowler; Arnd Bergmann > Subject: Re: [PATCH v4 0/5] zynq subarch cleanups > > On Mon, Oct 29, 2012 at 07:24:16AM +, Michal Simek wrote: > > Hi Josh, > > > > > Michal- > > > > > > Here is a v5 of the zynq cleanup patchset that addresses your > > > feedback. I've intentionally left patches 4 and 5 in the set until > > > we figure out the appropriate way to get them in tree (feel free to > > > just apply 1-3) > > > > I am ok to pick just several patches from your patchset. But this is > > no definitely good working style. Not expert for submission process > > but I think that if there is one broken patch maintainer shouldn't > > apply it. Can someone else check this? > > It turns out that with the change to patch 5 to map the uart to a known > working > address (instead of VMALLOC_END - 0x1000), patch 4 isn't needed, and as such > can be dropped. (I didn't realize this until this morning until I had saw > you had > applied 1-3,5 to your tree, but not 4). > > So, for what it's worth, you've applied all of the relevant patches for this > patchset. Ok. Great. Thanks, MIchal -- 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 v2] ARM: zynq: Allow UART1 to be used as DEBUG_LL console.
On 10/29/2012 07:19 PM, Nick Bowler wrote: The main UART on the Xilinx ZC702 board is UART1, located at address e0001000. Add a Kconfig option to select this device as the low-level debugging port. This allows the really early boot printouts to reach the USB serial adaptor on this board. For consistency's sake, add a choice entry for UART0 even though it is the the default if UART1 is not selected. Signed-off-by: Nick Bowler Tested-by: Josh Cartwright --- v2: rebase on newest patch series, signoff. This should apply cleanly on top of Josh Cartwright's v5 "zynq subarch cleanups" series. arch/arm/Kconfig.debug | 17 + arch/arm/mach-zynq/common.c|6 +++--- arch/arm/mach-zynq/include/mach/zynq_soc.h | 16 +++- 3 files changed, 31 insertions(+), 8 deletions(-) diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug index b0f3857b3a4c..7754d51f2b19 100644 --- a/arch/arm/Kconfig.debug +++ b/arch/arm/Kconfig.debug @@ -132,6 +132,23 @@ choice their output to UART1 serial port on DaVinci TNETV107X devices. + config DEBUG_ZYNQ_UART0 + bool "Kernel low-level debugging on Xilinx Zynq using UART0" + depends on ARCH_ZYNQ + help + Say Y here if you want the debug print routines to direct + their output to UART0 on the Zynq platform. + + config DEBUG_ZYNQ_UART1 + bool "Kernel low-level debugging on Xilinx Zynq using UART1" + depends on ARCH_ZYNQ + help + Say Y here if you want the debug print routines to direct + their output to UART1 on the Zynq platform. + + If you have a ZC702 board and want early boot messages to + appear on the USB serial adaptor, select this option. + config DEBUG_DC21285_PORT bool "Kernel low-level debugging messages via footbridge serial port" depends on FOOTBRIDGE diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c index ba8d14f78d4d..93b91059faab 100644 --- a/arch/arm/mach-zynq/common.c +++ b/arch/arm/mach-zynq/common.c @@ -84,9 +84,9 @@ static struct map_desc io_desc[] __initdata = { #ifdef CONFIG_DEBUG_LL { - .virtual= UART0_VIRT, - .pfn= __phys_to_pfn(UART0_PHYS), - .length = UART0_SIZE, + .virtual= LL_UART_VADDR, + .pfn= __phys_to_pfn(LL_UART_PADDR), + .length = UART_SIZE, .type = MT_DEVICE, }, #endif diff --git a/arch/arm/mach-zynq/include/mach/zynq_soc.h b/arch/arm/mach-zynq/include/mach/zynq_soc.h index 1b8bf0ecbcb0..7f4f38bcada9 100644 --- a/arch/arm/mach-zynq/include/mach/zynq_soc.h +++ b/arch/arm/mach-zynq/include/mach/zynq_soc.h @@ -25,8 +25,9 @@ * address that is known to work. */ #define UART0_PHYS0xE000 -#define UART0_SIZE SZ_4K -#define UART0_VIRT 0xF0001000 +#define UART1_PHYS 0xE0001000 +#define UART_SIZE SZ_4K +#define UART_VIRT 0xF0001000 #define TTC0_PHYS 0xF8001000 #define TTC0_SIZE SZ_4K @@ -36,12 +37,17 @@ #define SCU_PERIPH_SIZE SZ_8K #define SCU_PERIPH_VIRT (TTC0_VIRT - SCU_PERIPH_SIZE) +#if IS_ENABLED(CONFIG_DEBUG_ZYNQ_UART1) +# define LL_UART_PADDRUART1_PHYS +# define LL_UART_VADDRUART_VIRT +#else +# define LL_UART_PADDRUART0_PHYS +# define LL_UART_VADDRUART_VIRT +#endif Probably no reason to setup LL_UART_VADDR on two lines. It is enough to set it up once. MINOR: It is just my personal preference to use different coding style. #if IS_ENABLED(CONFIG_DEBUG_ZYNQ_UART1) # define LL_UART_PADDR UART1_PHYS #else # define LL_UART_PADDR UART0_PHYS #endif #define LL_UART_VADDR UART_VIRT Thanks, Michal -- Michal Simek, Ing. (M.Eng) w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/ Microblaze U-BOOT custodian -- 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] ARM: zynq: move ttc timer code to drivers/clocksource
On 10/29/2012 07:56 PM, Josh Cartwright wrote: Suggested cleanup by Arnd Bergmann. Move the ttc timer.c code to drivers/clocksource, and out of the mach-zynq directory. The common.h (which only held the timer declaration) was renamed to xilinx_ttc.h and moved into include/linux. Signed-off-by: Josh Cartwright Cc: Arnd Bergmann --- arch/arm/mach-zynq/Makefile | 2 +- arch/arm/mach-zynq/common.c | 2 +- arch/arm/mach-zynq/common.h | 24 arch/arm/mach-zynq/timer.c | 298 --- drivers/clocksource/Makefile | 1 + drivers/clocksource/xilinx_ttc.c | 297 ++ include/linux/xilinx_ttc.h | 24 7 files changed, 324 insertions(+), 324 deletions(-) delete mode 100644 arch/arm/mach-zynq/common.h delete mode 100644 arch/arm/mach-zynq/timer.c create mode 100644 drivers/clocksource/xilinx_ttc.c create mode 100644 include/linux/xilinx_ttc.h Really? If yes. shouldn't be there any better naming convention especially for headers. linux/clocksource/xilinx_ttc.h. Thanks, Michal -- Michal Simek, Ing. (M.Eng) w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/ Microblaze U-BOOT custodian -- 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 v4 4/5] ARM: annotate VMALLOC_END definition with _AC
> -Original Message- > From: Arnd Bergmann [mailto:a...@arndb.de] > Sent: Tuesday, October 30, 2012 11:22 PM > To: Michal Simek > Cc: Josh Cartwright; a...@kernel.org; linux-kernel@vger.kernel.org; linux-arm- > ker...@lists.infradead.org; John Linn; Nick Bowler; Russell King - ARM Linux > Subject: Re: [PATCH v4 4/5] ARM: annotate VMALLOC_END definition with _AC > > On Saturday 27 October 2012, Michal Simek wrote: > > > diff --git a/arch/arm/include/asm/pgtable.h > > > b/arch/arm/include/asm/pgtable.h index 08c1231..72904a2 100644 > > > --- a/arch/arm/include/asm/pgtable.h > > > +++ b/arch/arm/include/asm/pgtable.h > > > @@ -40,7 +40,7 @@ > > > */ > > > #define VMALLOC_OFFSET (8*1024*1024) > > > #define VMALLOC_START(((unsigned long)high_memory + > > > VMALLOC_OFFSET) & ~(VMALLOC_OFFSET-1)) > > > -#define VMALLOC_END 0xff00UL > > > +#define VMALLOC_END _AC(0xff00,UL) > > > > This shouldn't be the part of this series but should go to mainline through > different tree. > > Arnd, Olof: Can you take this patch to your arm-soc tree? > > > > I don't think it is a good workstyle to propose it to mainline through zynq > > soc > tree. > > What do you think? > > The arm-soc tree is not the right place either, this is architecture code > which is in > Russell's domain. I would suggest getting an Ack from Russell if he's ok with > it > and then merging it together with your other changes into arm-soc. That's what I thought too. Not sure if Josh wants to push this to mainline. Thanks, Michal This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately. -- 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] ARM: zynq: move ttc timer code to drivers/clocksource
> -Original Message- > From: Arnd Bergmann [mailto:a...@arndb.de] > Sent: Tuesday, October 30, 2012 11:37 PM > To: mon...@monstr.eu > Cc: Josh Cartwright; Michal Simek; linux-arm-ker...@lists.infradead.org; > linux- > ker...@vger.kernel.org; a...@kernel.org; Thomas Gleixner > Subject: Re: [PATCH] ARM: zynq: move ttc timer code to drivers/clocksource > > On Tuesday 30 October 2012, Michal Simek wrote: > > On 10/29/2012 07:56 PM, Josh Cartwright wrote: > > > Suggested cleanup by Arnd Bergmann. Move the ttc timer.c code to > > > drivers/clocksource, and out of the mach-zynq directory. > > > > > > The common.h (which only held the timer declaration) was renamed to > > > xilinx_ttc.h and moved into include/linux. > > > > > > Signed-off-by: Josh Cartwright > > > Cc: Arnd Bergmann > > > --- > > > arch/arm/mach-zynq/Makefile | 2 +- > > > arch/arm/mach-zynq/common.c | 2 +- > > > arch/arm/mach-zynq/common.h | 24 > > > arch/arm/mach-zynq/timer.c | 298 > > > --- > > > drivers/clocksource/Makefile | 1 + > > > drivers/clocksource/xilinx_ttc.c | 297 > ++ > > > include/linux/xilinx_ttc.h | 24 > > > 7 files changed, 324 insertions(+), 324 deletions(-) > > When you submit a patch that moves files around, please use the '-M' flag to > git- > format-patch so we can see the actual changes instead of a file being removed > and another one added. > > > > delete mode 100644 arch/arm/mach-zynq/common.h > > > delete mode 100644 arch/arm/mach-zynq/timer.c > > > create mode 100644 drivers/clocksource/xilinx_ttc.c > > > create mode 100644 include/linux/xilinx_ttc.h > > > > Really? > > If yes. shouldn't be there any better naming convention especially for > > headers. linux/clocksource/xilinx_ttc.h. > > Moving it is certainly the right direction, but I think we need a better way > to > handle those forward declarations. "struct sys_timer" is actually an ARM > specific > structure, so we might just want to add all the forward declarations for the > timers into arch/arm/include/asm/mach/time.h. It's not ideal to do it like > that, > but I think it's much better than having a new globally visible header for > each > timer that is used on ARM. > > Eventually, we might want to do something similar to what we are discussing > for > the top-level IRQ controllers at the moment, where we just autodetect them > from DT if possible, so we don't need to have any pointer to the timer from > arch > code at all. Ok. It means that it is not big deal to keep timer as is in mach and when we have any generic solution we can follow it. This patch just move the code out of mach-zynq. It should be done across architecture because for example timer I use for microblaze can be used by zynq too, also by Xilinx ppc. Thanks, Michal -- 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: [RFC 17/17] clk: zynq: remove call to of_clk_init
On 08/23/2013 09:32 AM, Steffen Trumtrar wrote: > Hi! > > On Thu, Aug 22, 2013 at 05:59:36PM -0700, Sören Brinkmann wrote: >> On Thu, Aug 22, 2013 at 05:26:47PM -0700, Sören Brinkmann wrote: >>> Hi Sebastian, >>> >>> On Tue, Aug 20, 2013 at 04:04:31AM +0200, Sebastian Hesselbarth wrote: >>>> With arch/arm calling of_clk_init(NULL) from time_init(), we can now >>>> remove it from corresponding drivers/clk code. >>> >>> I think that would break Zynq. >>> If I see this correctly you call of_clk_init() from common code, >>> _before_ the SOC specific time init function is called. >>> The problem is, that we have code setting up a global pointer which is >>> required by zynq_clk_setup() which is triggered when of_clk_init() is >>> called. >>> >>> Let me try to illustrate the current call graph: >>> >>> time_init() >>> zynq_timer_init() // this machines init_time() >>> zynq_slcr_init()// setup System Level Control Registers >>> including a global pointer >>> zynq_clock_init() >>> of_clk_init() >>> zynq_clk_setup() // requires pointer >>> setup in zynq_slcr_init() >>> ... >>> >>> IIUC, your series would change this to: >>> time_init() >>> of_clk_init() >>> zynq_clk_setup()// SLCR pointer is not setup/NULL >>> ... >>> zynq_timer_init() >>> zynq_slcr_init()// now the pointer becomes valid >> >> I guess we could move zynq_slcr_init() into init_irq(). I'll give that a >> shot tomorrow. >> > > I propose getting rid of the whole global pointer and let the clkc map the > address itself instead. > > Then there is no need to shuffle stuff around in the initcalls. > I have some WIP patches (not rebased on next and not even tested with it, > but with v3.11-rc4) > > The dtsi would be something like: > >control-register@f800 { This name is incorrect - it still should be slcr (system level control registers) > compatible = "simple-bus"; I expect that syscon compatible should be here not in the lock part because you want to map the whole reg space. >#address-cells = <1>; >#size-cells = <1>; >reg = <0xf800 0x1000>; >ranges; > >slcr: slcr@f800 { we should use different name here - lock/locks/etc. >compatible = "xlnx,zynq-slcr", "syscon"; >reg = <0xf800 0x10>; >}; > >clkc: clkc@f8000100 { >#clock-cells = <1>; >compatible = "xlnx,ps7-clkc"; >reg = <0xf8000100 0x100>; >ps-clk-frequency = <>; > xlnx,slcr = <&slcr>; Currently there is no code which handles locks that's why I think at least for now it is not necessary to extend binding which feature which is not used. >clock-output-names = "armpll", "ddrpll", "iopll", > "cpu_6or4x", >"cpu_3or2x", "cpu_2x", "cpu_1x", > "ddr2x", "ddr3x", >"dci", "lqspi", "smc", "pcap", "gem0", > "gem1", >"fclk0", "fclk1", "fclk2", "fclk3", > "can0", "can1", >"sdio0", "sdio1", "uart0", "uart1", > "spi0", "spi1", >"dma", "usb0_aper", "usb1_aper", > "gem0_aper", >"gem1_aper", "sdio0_aper", > "sdio1_aper", >"spi0_aper", "spi1_aper", "can0_aper", > "can1_aper", >"i2c0_aper", "i2c1_aper", > "uart0_aper", "uart1_aper", >"gpio_aper", "lqspi_aper", "smc_aper", > "swdt", >"dbg_trc", "dbg_apb"; >}; > >mio: pinmux@f8000700 { >compatible = "xlnx,ps7-pinctrl"; Have you created any driver for pinmuxing stuff? I agree with Soren - let's fix the current problem and then when Steffen has patches with syscon we can look at them. If there is any discussion about early syscon registration please let me know. 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: [RFC 17/17] clk: zynq: remove call to of_clk_init
On 08/26/2013 02:53 PM, Sebastian Hesselbarth wrote: > On 08/26/13 14:07, Steffen Trumtrar wrote: >> On Mon, Aug 26, 2013 at 01:15:11PM +0200, Michal Simek wrote: >>> I agree with Soren - let's fix the current problem and then when Steffen >>> has patches with syscon >>> we can look at them. >>> >>> If there is any discussion about early syscon registration please let me >>> know. >>> >> >> Where I'm stuck at the moment is: if I map the whole register space to the >> parent node, how do I get its mapped address in the clkc? > > Steffen, > > if slcr is such an essential part of the SoC, you can choose to provide > zynq_slcr_readl/writel callbacks. You can then use those callback in the > clock driver without knowing the base address. Also, it allows you to > hide slcr specific locking details from subsequent drivers using the > callbacks. I don't think this will help. What you need to call in the clk driver is regmap_read/regmap_write but you can call it when syscon/regmap driver is initialized. Steffen: Can you point me to that floading patches? If they are useful we can try them and help with pushing them to the mainline. I don't think that there is any reasonable solution without using these patches. 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 RFC v2 16/16] ARM: zynq: Don't call of_clk_init()
On 08/29/2013 03:37 PM, Arnd Bergmann wrote: > On Tuesday 27 August 2013, Sebastian Hesselbarth wrote: >> @@ -58,10 +57,10 @@ static void __init zynq_init_machine(void) >> of_platform_bus_probe(NULL, zynq_of_bus_ids, NULL); >> } >> >> -static void __init zynq_timer_init(void) >> +static void __init zynq_init_irq(void) >> { >> + irqchip_init(); >> zynq_slcr_init(); >> - clocksource_of_init(); >> } >> >> static struct map_desc zynq_cortex_a9_scu_map __initdata = { >> @@ -104,8 +103,8 @@ static const char * const zynq_dt_match[] = { >> DT_MACHINE_START(XILINX_EP107, "Xilinx Zynq Platform") >> .smp= smp_ops(zynq_smp_ops), >> .map_io = zynq_map_io, >> + .init_irq = zynq_init_irq, >> .init_machine = zynq_init_machine, >> - .init_time = zynq_timer_init, >> .dt_compat = zynq_dt_match, >> .restart= zynq_system_reset, >> MACHINE_END > > It looks like we are not getting any closer to removing all callbacks here, > since you add one in order to remove another, and after the patch we do > more things "early", which we try to avoid. I think we're better off without > this particular patch. Is there any plan to remove all of them? I expect that on almost all platforms it is a need to have at least one early hook to be able to setup things. Thanks, Michal -- 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: [RFC PATCH] fpga: Introduce new fpga subsystem
On 09/30/2013 07:12 PM, Jason Gunthorpe wrote: > On Fri, Sep 27, 2013 at 03:31:53PM +0200, Michal Simek wrote: > >> I expect that you are detecting/specifying in the driver which >> fpga is connected and you also need to know size of this device. >> Then your driver allocate buffer with this size in the kernel >> and streming data to this buffer. When this is done you are >> using another sysfs files to control device programming. > > No, it just streams: > > static ssize_t fpga_config_write(struct file *filp,struct kobject *kobj, > struct bin_attribute *attr, > char *buf, loff_t off, size_t len) > { > struct device *dev = container_of(kobj, struct device, kobj); > struct platform_device *pdev = to_platform_device(dev); > struct fpga_priv *priv = platform_get_drvdata(pdev); > uint8_t *cur = buf; > size_t total = len; > unsigned int bit; > > for (; len != 0; len--, cur++) { > gpio_set_value(priv->gpio[GPIO_CCLK],0); > > for (bit = 0; bit != 8; bit++) > gpio_set_value(priv->data_gpio[bit], >(*cur & (1< > gpio_set_value(priv->gpio[GPIO_CCLK],1); > > if (gpio_get_value(priv->gpio[GPIO_INIT_B]) == 0) > return -EIO; > } > return total; > } > > static struct bin_attribute dev_attr_config_data = { > .attr = { > .name = "config_data", > .mode = 0600, > }, > .size = 0, > .write = fpga_config_write, > }; > > User space does as many writes as necessary to send the entire > bitstream, the sysfs layer chunks things into PAGE_SIZE blocks, so it > acts much like a socket with O_NONBLOCK set. > > We are controlling the other related GPIOs from userspace, but for > your purposes I would pair the data sysfs file with a control sysfs > file much like request firwmare does. > > Here is a suggestion. > - Two files fpga_config_state, fpga_config_data > - fpga_config_state is a one value text string values are like >initializing, clearing, programming, operating, error_clear_failed, >error_bistream_crc > - Userspace writes to fpga_config_state which causes the kernel FSM > to move to that state. The normal progression would be initializing, > clearing, programming and finally operating > - The kernel can move to an error_* state if it detects a problem > - The programming state data from fpga_config_data to the > configuration bus and userspace writes 'operating' once the stream > is done to perform the post-configuration actions. yes, there is necessary to provide also any state to be able to control the flow. For your case above with streams this is not necessary. Thanks for this description I wanted to make sure that we are on the same page. 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
[RFC PATCH v2 0/1] FPGA subsystem core
Hi All, this is the second attempt to introduce new Linux FPGA subsystem which can help us to unify all fpga drivers which in general do the same things. Xilinx has hwicap in the kernel as char driver (drivers/char/xilinx_hwicap/) and I would like to base Zynq devcfg driver based on this interface because make no sense to push the Linux kernel another char driver (I am testing this interface on this driver). Based on my discussion at ELC with Greg KH the new driver should support firmware interface for loading bitstream. FPGA manager/driver just define set of functions and call fpga_mgr_register(). struct fpga_manager_ops zynq_fpga_mgr_ops = { .read_init = zynq_init, .write_init = zynq_init, .read = zynq_read, .write = zynq_write, .read_complete = zynq_complete, .write_complete = zynq_complete, }; fpga_mgr_register(pdev, &zynq_fpga_mgr_ops, "Zynq FPGA Manager", priv); For unregistration it is enough to call: fpga_mgr_unregister(pdev); Here is the set of commands for writing bitstream to FPGA. Through firmware interface: cat /sys/class/fpga_manager/fpga0/name echo -n fpga.bin > /sys/class/fpga_manager/fpga0/firmware Through sysfs bin file: cat /sys/class/fpga_manager/fpga0/fpga_config_state echo -n write_init > /sys/class/fpga_manager/fpga0/fpga_config_state cat /lib/firmware/fpga.bin > /sys/class/fpga_manager/fpga0/fpga_config_data echo -n write_complete > /sys/class/fpga_manager/fpga0/fpga_config_state Subsystem supports working with phandles for cases where you want to load bitstreams for particular device through defined device. For example: mngr@0 { compatible = "whatever"; fpga-mgr = <&ps7_dev_cfg_0>; ... } ; With these lines you can get easily load bitstream to the device. struct fpga_manager *mgr; mgr = of_find_fpga_mgr_by_phandle(pdev->dev.of_node, "fpga-mgr"); if (mgr) mgr->fpga_firmware_write(mgr, "filename"); NOTE: I have added there of_find_fpga_mgr_by_node() and of_find_fpga_mgr_by_phandle() but maybe they should be added separately to drivers/of/of_fpga.c. Alessandro: I haven't looked at your FMC cases but maybe this could be also worth for your cases. TODO: - Probably make sense to create doc in Documentation folder too. - When interface is fine also send zynq devcfg driver - Properly test reading (we have problem with zynq devcfg driver now) - Not sure if firmware interface also provide option to create files from kernel space. Thanks for your comments, Michal Changes in v2: - Remove ! from all error message not to be shouty - Fix error codes - Add sysfs-class-fpga description - Use read/write helper functions with bit protection - s/fpga_mgr_status_show/fpga_mgr_status_read/g - Do not all end driver status just show core status - Extract firmware support to specific sysfs firmware file - Add support for sysfs bin attributes (fpga_config_state, fpga_config_data) - Allocate space for name dynamically - Introduce new flags bits (INIT_DONE, READ, WRITE) Michal Simek (1): fpga: Introduce new fpga subsystem Documentation/ABI/testing/sysfs-class-fpga | 33 ++ MAINTAINERS| 7 + drivers/Kconfig| 2 + drivers/Makefile | 1 + drivers/fpga/Kconfig | 18 + drivers/fpga/Makefile | 5 + drivers/fpga/fpga-mgr.c| 753 + include/linux/fpga.h | 110 + 8 files changed, 929 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-class-fpga create mode 100644 drivers/fpga/Kconfig create mode 100644 drivers/fpga/Makefile create mode 100644 drivers/fpga/fpga-mgr.c create mode 100644 include/linux/fpga.h -- 1.8.2.3 pgpjMHYFmMlQv.pgp Description: PGP signature
[RFC PATCH v2] fpga: Introduce new fpga subsystem
This new fpga subsystem core should unify all fpga drivers/managers which do the same things. Load configuration data to fpga or another programmable logic through common interface. It doesn't matter if it is MMIO device, gpio bitbanging, etc. connection. The point is to have the same interface for these drivers. Signed-off-by: Michal Simek --- Changes in v2: - Remove ! from all error message not to be shouty - Fix error codes - Add sysfs-class-fpga description - Use read/write helper functions with bit protection - s/fpga_mgr_status_show/fpga_mgr_status_read/g - Do not all end driver status just show core status - Extract firmware support to specific sysfs firmware file - Add support for sysfs bin attributes (fpga_config_state, fpga_config_data) - Allocate space for name dynamically - Introduce new flags bits (INIT_DONE, READ, WRITE) Documentation/ABI/testing/sysfs-class-fpga | 33 ++ MAINTAINERS| 7 + drivers/Kconfig| 2 + drivers/Makefile | 1 + drivers/fpga/Kconfig | 18 + drivers/fpga/Makefile | 5 + drivers/fpga/fpga-mgr.c| 753 + include/linux/fpga.h | 110 + 8 files changed, 929 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-class-fpga create mode 100644 drivers/fpga/Kconfig create mode 100644 drivers/fpga/Makefile create mode 100644 drivers/fpga/fpga-mgr.c create mode 100644 include/linux/fpga.h diff --git a/Documentation/ABI/testing/sysfs-class-fpga b/Documentation/ABI/testing/sysfs-class-fpga new file mode 100644 index 000..3d66c71 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-class-fpga @@ -0,0 +1,33 @@ +What: /sys/class/fpga_manager/fpga/firmware +Date: October 2013 +KernelVersion: 3.12 +Contact: Michal Simek +Description: + Writing firmware name will invoke firmware interface + to request for a firmware which includes fpga bitstream. + +What: /sys/class/fpga_manager/fpga/fpga_config_state +Date: October 2013 +KernelVersion: 3.12 +Contact: Michal Simek +Description: + By reading this file you will get current fpga manager state. + Flag bits are present in include/linux/fpga.h (FPGA_MGR_XX). + By writing to this file you can change fpga manager state. + Valid options are: write_init, write_complete, read_init, + read_complete. + +What: /sys/class/fpga_manager/fpga/fpga_config_data +Date: October 2013 +KernelVersion: 3.12 +Contact: Michal Simek +Description: + For writing fpga bitstream to the device when device is + in proper state setup through fpga_config_state. + +What: /sys/class/fpga_manager/fpga/name +Date: October 2013 +KernelVersion: 3.12 +Contact: Michal Simek +Description: + Show fpga manager name. diff --git a/MAINTAINERS b/MAINTAINERS index e61c2e8..5c7597b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3427,6 +3427,13 @@ F: include/linux/fmc*.h F: include/linux/ipmi-fru.h K: fmc_d.*register +FPGA SUBSYSTEM +M: Michal Simek +T: git git://git.xilinx.com/linux-xlnx.git +S: Supported +F: drivers/fpga/ +F: include/linux/fpga.h + FPU EMULATOR M: Bill Metzenthen W: http://floatingpoint.sourceforge.net/emulator/index.html diff --git a/drivers/Kconfig b/drivers/Kconfig index aa43b91..17f3caa 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -166,4 +166,6 @@ source "drivers/reset/Kconfig" source "drivers/fmc/Kconfig" +source "drivers/fpga/Kconfig" + endmenu diff --git a/drivers/Makefile b/drivers/Makefile index ab93de8..2b5e73b 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -152,3 +152,4 @@ obj-$(CONFIG_VME_BUS) += vme/ obj-$(CONFIG_IPACK_BUS)+= ipack/ obj-$(CONFIG_NTB) += ntb/ obj-$(CONFIG_FMC) += fmc/ +obj-$(CONFIG_FPGA) += fpga/ diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig new file mode 100644 index 000..5357645 --- /dev/null +++ b/drivers/fpga/Kconfig @@ -0,0 +1,18 @@ +# +# FPGA framework configuration +# + +menu "FPGA devices" + +config FPGA + tristate "FPGA Framework" + help + Say Y here if you want support for configuring FPGAs from the + kernel. The FPGA framework adds a FPGA manager class and FPGA + manager drivers. + +if FPGA + +endif + +endmenu diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile new file mode 100644 index 000..3c7f29b --- /dev/null +++ b/drivers/fpga/Makefile @@ -0,0 +1,5 @@ +# +# Makefile for the fpga framework and fpga manager drivers. +# + +obj-$(CONFIG_FPGA) += fpga-mgr.o diff --git a/drivers/fpga/fp
Re: [PATCH] amba: Ensure drvdata is NULL
On 10/02/2013 10:25 PM, Russell King - ARM Linux wrote: > On Mon, Sep 30, 2013 at 08:59:06AM +0200, Michal Simek wrote: >> This patch is inpired by the patch for drvdata >> "device-core: Ensure drvdata = NULL when no driver is bound" >> (sha1: 0998d0631001288a5974afc0b2a5f568bcdecb4d) >> >> Also it fixes all occurences in drivers. >> >> Signed-off-by: Michal Simek >> --- >> This patch has been sent as RFC in this thread. >> http://lkml.org/lkml/2013/9/4/393 > > Why not have the driver core do this? Then it gets applied to all bus > types uniformly. ok - I have checked that path and it should be already done in really_probe function which calls amba_probe() and then driver probe function. It means that the patch should contain just "amba_set_drvdata(dev, NULL);" removal and not touching bus.c file. Thanks, Michal -- 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/
[PATCH 2/9] input: ambakmi: Remove unnecessary amba_set_drvdata()
Driver core clears the driver data to NULL after device_release or on probe failure, so just remove it from here. Driver core change: "device-core: Ensure drvdata = NULL when no driver is bound" (sha1: 0998d0631001288a5974afc0b2a5f568bcdecb4d) Signed-off-by: Michal Simek --- drivers/input/serio/ambakmi.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/input/serio/ambakmi.c b/drivers/input/serio/ambakmi.c index 4e2fd44..b7c206d 100644 --- a/drivers/input/serio/ambakmi.c +++ b/drivers/input/serio/ambakmi.c @@ -167,8 +167,6 @@ static int amba_kmi_remove(struct amba_device *dev) { struct amba_kmi_port *kmi = amba_get_drvdata(dev); - amba_set_drvdata(dev, NULL); - serio_unregister_port(kmi->io); clk_put(kmi->clk); iounmap(kmi->base); -- 1.8.2.3 pgpsdWOZjQXj_.pgp Description: PGP signature
[PATCH 1/9] dma: pl330: Remove unnecessary amba_set_drvdata()
Driver core clears the driver data to NULL after device_release or on probe failure, so just remove it from here. Driver core change: "device-core: Ensure drvdata = NULL when no driver is bound" (sha1: 0998d0631001288a5974afc0b2a5f568bcdecb4d) Signed-off-by: Michal Simek --- drivers/dma/pl330.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c index a562d24..dfb2931 100644 --- a/drivers/dma/pl330.c +++ b/drivers/dma/pl330.c @@ -3029,8 +3029,6 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id) return 0; probe_err3: - amba_set_drvdata(adev, NULL); - /* Idle the DMAC */ list_for_each_entry_safe(pch, _p, &pdmac->ddma.channels, chan.device_node) { @@ -3064,7 +3062,6 @@ static int pl330_remove(struct amba_device *adev) of_dma_controller_free(adev->dev.of_node); dma_async_device_unregister(&pdmac->ddma); - amba_set_drvdata(adev, NULL); /* Idle the DMAC */ list_for_each_entry_safe(pch, _p, &pdmac->ddma.channels, -- 1.8.2.3 pgpCxAO8Yfa_E.pgp Description: PGP signature
[PATCH 7/9] spi: pl022: Remove unnecessary amba_set_drvdata()
Driver core clears the driver data to NULL after device_release or on probe failure, so just remove it from here. Driver core change: "device-core: Ensure drvdata = NULL when no driver is bound" (sha1: 0998d0631001288a5974afc0b2a5f568bcdecb4d) Signed-off-by: Michal Simek --- drivers/spi/spi-pl022.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c index 9c511a9..f661a7e 100644 --- a/drivers/spi/spi-pl022.c +++ b/drivers/spi/spi-pl022.c @@ -2288,7 +2288,6 @@ pl022_remove(struct amba_device *adev) amba_release_regions(adev); tasklet_disable(&pl022->pump_transfers); spi_unregister_master(pl022->master); - amba_set_drvdata(adev, NULL); return 0; } -- 1.8.2.3 pgpG5RVYmRo1U.pgp Description: PGP signature
[PATCH 9/9] video: amba-clcd: Remove unnecessary amba_set_drvdata()
Driver core clears the driver data to NULL after device_release or on probe failure, so just remove it from here. Driver core change: "device-core: Ensure drvdata = NULL when no driver is bound" (sha1: 0998d0631001288a5974afc0b2a5f568bcdecb4d) Signed-off-by: Michal Simek --- drivers/video/amba-clcd.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/video/amba-clcd.c b/drivers/video/amba-clcd.c index 0a2cce7..0bab6ab 100644 --- a/drivers/video/amba-clcd.c +++ b/drivers/video/amba-clcd.c @@ -594,8 +594,6 @@ static int clcdfb_remove(struct amba_device *dev) { struct clcd_fb *fb = amba_get_drvdata(dev); - amba_set_drvdata(dev, NULL); - clcdfb_disable(fb); unregister_framebuffer(&fb->fb); if (fb->fb.cmap.len) -- 1.8.2.3 pgpoFWrKjuJhO.pgp Description: PGP signature
[PATCH 8/9] watchdog: sp805_wdt: Remove unnecessary amba_set_drvdata()
Driver core clears the driver data to NULL after device_release or on probe failure, so just remove it from here. Driver core change: "device-core: Ensure drvdata = NULL when no driver is bound" (sha1: 0998d0631001288a5974afc0b2a5f568bcdecb4d) Signed-off-by: Michal Simek --- drivers/watchdog/sp805_wdt.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c index 58df98a..3f786ce 100644 --- a/drivers/watchdog/sp805_wdt.c +++ b/drivers/watchdog/sp805_wdt.c @@ -268,7 +268,6 @@ static int sp805_wdt_remove(struct amba_device *adev) struct sp805_wdt *wdt = amba_get_drvdata(adev); watchdog_unregister_device(&wdt->wdd); - amba_set_drvdata(adev, NULL); watchdog_set_drvdata(&wdt->wdd, NULL); return 0; -- 1.8.2.3 pgp6FmnULyBkd.pgp Description: PGP signature
[PATCH 6/9] serial: Remove unnecessary amba_set_drvdata()
Driver core clears the driver data to NULL after device_release or on probe failure, so just remove it from here. Driver core change: "device-core: Ensure drvdata = NULL when no driver is bound" (sha1: 0998d0631001288a5974afc0b2a5f568bcdecb4d) Signed-off-by: Michal Simek --- drivers/tty/serial/amba-pl010.c | 3 --- drivers/tty/serial/amba-pl011.c | 3 --- 2 files changed, 6 deletions(-) diff --git a/drivers/tty/serial/amba-pl010.c b/drivers/tty/serial/amba-pl010.c index 8b90f0b..33bd860 100644 --- a/drivers/tty/serial/amba-pl010.c +++ b/drivers/tty/serial/amba-pl010.c @@ -728,7 +728,6 @@ static int pl010_probe(struct amba_device *dev, const struct amba_id *id) amba_set_drvdata(dev, uap); ret = uart_add_one_port(&amba_reg, &uap->port); if (ret) { - amba_set_drvdata(dev, NULL); amba_ports[i] = NULL; clk_put(uap->clk); unmap: @@ -745,8 +744,6 @@ static int pl010_remove(struct amba_device *dev) struct uart_amba_port *uap = amba_get_drvdata(dev); int i; - amba_set_drvdata(dev, NULL); - uart_remove_one_port(&amba_reg, &uap->port); for (i = 0; i < ARRAY_SIZE(amba_ports); i++) diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c index aaa2286..7203864 100644 --- a/drivers/tty/serial/amba-pl011.c +++ b/drivers/tty/serial/amba-pl011.c @@ -2147,7 +2147,6 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id) amba_set_drvdata(dev, uap); ret = uart_add_one_port(&amba_reg, &uap->port); if (ret) { - amba_set_drvdata(dev, NULL); amba_ports[i] = NULL; pl011_dma_remove(uap); } @@ -2160,8 +2159,6 @@ static int pl011_remove(struct amba_device *dev) struct uart_amba_port *uap = amba_get_drvdata(dev); int i; - amba_set_drvdata(dev, NULL); - uart_remove_one_port(&amba_reg, &uap->port); for (i = 0; i < ARRAY_SIZE(amba_ports); i++) -- 1.8.2.3 pgpkrNq20yRcE.pgp Description: PGP signature
[PATCH 5/9] rtc: amba: Remove unnecessary amba_set_drvdata()
Driver core clears the driver data to NULL after device_release or on probe failure, so just remove it from here. Driver core change: "device-core: Ensure drvdata = NULL when no driver is bound" (sha1: 0998d0631001288a5974afc0b2a5f568bcdecb4d) Signed-off-by: Michal Simek --- drivers/rtc/rtc-pl030.c | 2 -- drivers/rtc/rtc-pl031.c | 2 -- 2 files changed, 4 deletions(-) diff --git a/drivers/rtc/rtc-pl030.c b/drivers/rtc/rtc-pl030.c index 22bacdb..a804f75 100644 --- a/drivers/rtc/rtc-pl030.c +++ b/drivers/rtc/rtc-pl030.c @@ -153,8 +153,6 @@ static int pl030_remove(struct amba_device *dev) { struct pl030_rtc *rtc = amba_get_drvdata(dev); - amba_set_drvdata(dev, NULL); - writel(0, rtc->base + RTC_CR); free_irq(dev->irq[0], rtc); diff --git a/drivers/rtc/rtc-pl031.c b/drivers/rtc/rtc-pl031.c index 0f0609b..c9ca86e 100644 --- a/drivers/rtc/rtc-pl031.c +++ b/drivers/rtc/rtc-pl031.c @@ -305,7 +305,6 @@ static int pl031_remove(struct amba_device *adev) { struct pl031_local *ldata = dev_get_drvdata(&adev->dev); - amba_set_drvdata(adev, NULL); free_irq(adev->irq[0], ldata); rtc_device_unregister(ldata->rtc); iounmap(ldata->base); @@ -392,7 +391,6 @@ out_no_irq: rtc_device_unregister(ldata->rtc); out_no_rtc: iounmap(ldata->base); - amba_set_drvdata(adev, NULL); out_no_remap: kfree(ldata); out: -- 1.8.2.3 pgpM82_xlbEL1.pgp Description: PGP signature
[PATCH 3/9] ARM: etm: Remove unnecessary amba_set_drvdata()
Driver core clears the driver data to NULL after device_release or on probe failure, so just remove it from here. Driver core change: "device-core: Ensure drvdata = NULL when no driver is bound" (sha1: 0998d0631001288a5974afc0b2a5f568bcdecb4d) Signed-off-by: Michal Simek --- arch/arm/kernel/etm.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/arch/arm/kernel/etm.c b/arch/arm/kernel/etm.c index 8ff0ecd..131a6ab 100644 --- a/arch/arm/kernel/etm.c +++ b/arch/arm/kernel/etm.c @@ -385,7 +385,6 @@ out: return ret; out_unmap: - amba_set_drvdata(dev, NULL); iounmap(t->etb_regs); out_release: @@ -398,8 +397,6 @@ static int etb_remove(struct amba_device *dev) { struct tracectx *t = amba_get_drvdata(dev); - amba_set_drvdata(dev, NULL); - iounmap(t->etb_regs); t->etb_regs = NULL; @@ -588,7 +585,6 @@ out: return ret; out_unmap: - amba_set_drvdata(dev, NULL); iounmap(t->etm_regs); out_release: @@ -601,8 +597,6 @@ static int etm_remove(struct amba_device *dev) { struct tracectx *t = amba_get_drvdata(dev); - amba_set_drvdata(dev, NULL); - iounmap(t->etm_regs); t->etm_regs = NULL; -- 1.8.2.3 pgpAIS0M9Ufto.pgp Description: PGP signature
[PATCH 4/9] mmc: mmci: Remove unnecessary amba_set_drvdata()
Driver core clears the driver data to NULL after device_release or on probe failure, so just remove it from here. Driver core change: "device-core: Ensure drvdata = NULL when no driver is bound" (sha1: 0998d0631001288a5974afc0b2a5f568bcdecb4d) Signed-off-by: Michal Simek --- drivers/mmc/host/mmci.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index c3785ed..07e17f1 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -1678,8 +1678,6 @@ static int mmci_remove(struct amba_device *dev) { struct mmc_host *mmc = amba_get_drvdata(dev); - amba_set_drvdata(dev, NULL); - if (mmc) { struct mmci_host *host = mmc_priv(mmc); -- 1.8.2.3 pgphIDnewLF_U.pgp Description: PGP signature
Re: [PATCH v2 3/3] i2c: xilinx: Use devm_* functions
On 10/04/2013 07:33 AM, Wolfram Sang wrote: > >> +i2c->base = devm_ioremap_resource(&pdev->dev, res); >> +if (IS_ERR(i2c->base)) { >> +dev_err(&pdev->dev, "Could not allocate iomem\n"); > > devm_ioremap_resource already prints error messages. you are right. > >> +ret = devm_request_irq(&pdev->dev, irq, xiic_isr, 0, pdev->name, i2c); > > This is too early. Can you find out why? Why do you think that it is too early? I am looking at origin code again and I think that the code is also problematic because in xiic_reinit() interrupts are enabled but they are requested later. Shouldn't be there a logic that interrupts should be enabled when interrupts are registered by the kernel? >> +pdata = (struct xiic_i2c_platform_data *)dev_get_platdata(&pdev->dev); > > Casting a void pointer? No problem with that. 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 v2 2/3] i2c: xilinx: Set tx direction in write operation
On 10/04/2013 07:46 AM, Wolfram Sang wrote: > >> +cr = xiic_getreg32(i2c, XIIC_CR_REG_OFFSET); >> +cr |= XIIC_CR_DIR_IS_TX_MASK; >> +xiic_setreg32(i2c, XIIC_CR_REG_OFFSET, cr); >> + > > Is there no need to clear the bit again when receiving? This bit is cleared in xiic_xfer() -> xiic_start_xfer() ->xiic_reinit() xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_TX_FIFO_RESET_MASK); > And did > transferring ever work if this bit was never set before? I really don't know. We have switched from old driver to this new mainline one and based on our eeprom testing we have found that this bit hasn't been setup properly. It is described here. http://www.xilinx.com/support/documentation/ip_documentation/axi_iic/v1_02_a/axi_iic_ds756.pdf page 28 - step 3. IIC Master Transmitter with a Repeated Start 1. Write the IIC device address to the TX_FIFO. 2. Write data to TX_FIFO. 3. Write to Control Register (CR) to set MSMS = 1 and TX = 1. 4. Continue writing data to TX_FIFO. 5. Wait for transmit FIFO empty interrupt. This implies the IIC has throttled the bus. 6. Write to CR to set RSTA = 1. 7. Write IIC device address to TX_FIFO. 8. Write all data except last byte to TX_FIFO. 9. Wait for transmit FIFO empty interrupt. This implies the IIC has throttled the bus. 10. Write to CR to set MSMS = 0. The IIC generates a stop condition at the end of the last byte. 11. Write last byte of data to TX_FIFO. 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 v2 3/3] i2c: xilinx: Use devm_* functions
On 10/04/2013 01:58 PM, Wolfram Sang wrote: > On Fri, Oct 04, 2013 at 11:16:20AM +0200, Michal Simek wrote: >> On 10/04/2013 07:33 AM, Wolfram Sang wrote: >>> >>>> + i2c->base = devm_ioremap_resource(&pdev->dev, res); >>>> + if (IS_ERR(i2c->base)) { >>>> + dev_err(&pdev->dev, "Could not allocate iomem\n"); >>> >>> devm_ioremap_resource already prints error messages. >> >> you are right. >> >>> >>>> + ret = devm_request_irq(&pdev->dev, irq, xiic_isr, 0, pdev->name, i2c); >>> >>> This is too early. Can you find out why? >> >> Why do you think that it is too early? > > The ISR uses spinlocks which are not initialized by then. And waitqueue too. Ok I will keep that request irq in current location and I will add xiic_reinit() below this code. >> I am looking at origin code again and I think that the code >> is also problematic because in xiic_reinit() interrupts are enabled >> but they are requested later. >> Shouldn't be there a logic that interrupts should be enabled when >> interrupts are registered by the kernel? > > First register the handler, then activate interrupts. You are right, > this needs to be fixed, too. Do you want me to create separate patch just about moving request irq in front of xiic_reinit()? And then devm_ conversion? Thanks, Michal signature.asc Description: OpenPGP digital signature
Re: [PATCH v2 2/3] i2c: xilinx: Set tx direction in write operation
On 10/04/2013 02:12 PM, Lars-Peter Clausen wrote: > On 10/04/2013 01:55 PM, Wolfram Sang wrote: >> On Fri, Oct 04, 2013 at 11:53:49AM +0200, Michal Simek wrote: >>> On 10/04/2013 07:46 AM, Wolfram Sang wrote: >>>> >>>>> + cr = xiic_getreg32(i2c, XIIC_CR_REG_OFFSET); >>>>> + cr |= XIIC_CR_DIR_IS_TX_MASK; >>>>> + xiic_setreg32(i2c, XIIC_CR_REG_OFFSET, cr); >>>>> + >>>> >>>> Is there no need to clear the bit again when receiving? >>> >>> This bit is cleared in xiic_xfer() -> xiic_start_xfer() ->xiic_reinit() >>> >>> xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_TX_FIFO_RESET_MASK); >> >> A bit implicit, but OK. >> >>>> And did >>>> transferring ever work if this bit was never set before? >>> >>> I really don't know. We have switched from old driver to this new mainline >>> one >>> and based on our eeprom testing we have found that this bit hasn't been >>> setup properly. >>> >>> It is described here. >>> http://www.xilinx.com/support/documentation/ip_documentation/axi_iic/v1_02_a/axi_iic_ds756.pdf >>> page 28 - step 3. >>> >>> IIC Master Transmitter with a Repeated Start >>> 1. Write the IIC device address to the TX_FIFO. >>> 2. Write data to TX_FIFO. >>> 3. Write to Control Register (CR) to set MSMS = 1 and TX = 1. >>> 4. Continue writing data to TX_FIFO. >>> 5. Wait for transmit FIFO empty interrupt. This implies the IIC has >>> throttled the bus. >>> 6. Write to CR to set RSTA = 1. >> >> Repeated start is not happening in the driver as well, or am I >> overlooking something? >> >>> 7. Write IIC device address to TX_FIFO. >>> 8. Write all data except last byte to TX_FIFO. >>> 9. Wait for transmit FIFO empty interrupt. This implies the IIC has >>> throttled the bus. >>> 10. Write to CR to set MSMS = 0. The IIC generates a stop condition at the >>> end of the last byte. >>> 11. Write last byte of data to TX_FIFO. >> >> CCing more people who worked on the driver in the past and might have >> experiences > > The current version works fine here. The driver uses whats described in the > datasheet as "dynamic controller logic flow" and not the "standard > controller logic flow". The sequence Michal mentioned above is from the > "standard controller logic flow" section. Does this change break "dynamic controller logic flow"? 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 v2 2/3] i2c: xilinx: Set tx direction in write operation
On 10/04/2013 03:38 PM, Lars-Peter Clausen wrote: > On 10/04/2013 03:09 PM, Michal Simek wrote: >> >> >> On 10/04/2013 02:12 PM, Lars-Peter Clausen wrote: >>> On 10/04/2013 01:55 PM, Wolfram Sang wrote: >>>> On Fri, Oct 04, 2013 at 11:53:49AM +0200, Michal Simek wrote: >>>>> On 10/04/2013 07:46 AM, Wolfram Sang wrote: >>>>>> >>>>>>> + cr = xiic_getreg32(i2c, XIIC_CR_REG_OFFSET); >>>>>>> + cr |= XIIC_CR_DIR_IS_TX_MASK; >>>>>>> + xiic_setreg32(i2c, XIIC_CR_REG_OFFSET, cr); >>>>>>> + >>>>>> >>>>>> Is there no need to clear the bit again when receiving? >>>>> >>>>> This bit is cleared in xiic_xfer() -> xiic_start_xfer() ->xiic_reinit() >>>>> >>>>> xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_TX_FIFO_RESET_MASK); >>>> >>>> A bit implicit, but OK. >>>> >>>>>> And did >>>>>> transferring ever work if this bit was never set before? >>>>> >>>>> I really don't know. We have switched from old driver to this new >>>>> mainline one >>>>> and based on our eeprom testing we have found that this bit hasn't been >>>>> setup properly. >>>>> >>>>> It is described here. >>>>> http://www.xilinx.com/support/documentation/ip_documentation/axi_iic/v1_02_a/axi_iic_ds756.pdf >>>>> page 28 - step 3. >>>>> >>>>> IIC Master Transmitter with a Repeated Start >>>>> 1. Write the IIC device address to the TX_FIFO. >>>>> 2. Write data to TX_FIFO. >>>>> 3. Write to Control Register (CR) to set MSMS = 1 and TX = 1. >>>>> 4. Continue writing data to TX_FIFO. >>>>> 5. Wait for transmit FIFO empty interrupt. This implies the IIC has >>>>> throttled the bus. >>>>> 6. Write to CR to set RSTA = 1. >>>> >>>> Repeated start is not happening in the driver as well, or am I >>>> overlooking something? >>>> >>>>> 7. Write IIC device address to TX_FIFO. >>>>> 8. Write all data except last byte to TX_FIFO. >>>>> 9. Wait for transmit FIFO empty interrupt. This implies the IIC has >>>>> throttled the bus. >>>>> 10. Write to CR to set MSMS = 0. The IIC generates a stop condition at >>>>> the end of the last byte. >>>>> 11. Write last byte of data to TX_FIFO. >>>> >>>> CCing more people who worked on the driver in the past and might have >>>> experiences >>> >>> The current version works fine here. The driver uses whats described in the >>> datasheet as "dynamic controller logic flow" and not the "standard >>> controller logic flow". The sequence Michal mentioned above is from the >>> "standard controller logic flow" section. >> >> Does this change break "dynamic controller logic flow"? > > Not sure, but I would assume that the bit is ignored in this mode. But I > don't think the patch should be applied since this step is not in the > sequence of steps that should be done. Kedar: Can you please look at both these modes and provide feedback? 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: [RFC PATCH v2 0/1] FPGA subsystem core
On 10/03/2013 08:49 AM, Pavel Machek wrote: > On Wed 2013-10-02 12:00:52, H. Peter Anvin wrote: >> On 10/02/2013 08:35 AM, Michal Simek wrote: >>> >>> Based on my discussion at ELC with Greg KH the new driver should >>> support firmware interface for loading bitstream. >>> >> >> As I have previously stated, I think this is a mistake simply because >> the firmware interface is a bad mapping on requirements for an FPGA, >> especially once you account for the vast number of ways an FPGA can >> get loaded and you take partial reconfiguration into account. I >> happen to be at a face to face meeting with Greg today, so I'll pick >> his brain a bit. > > Umm. Could the discussion be kept on line? I agree with you. But anyway what was resolution from that meeting? 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: [RFC PATCH v2 0/1] FPGA subsystem core
On 10/04/2013 04:21 PM, H. Peter Anvin wrote: > Yes; I never got too corner Greg ;) > > Greg Kroah-Hartman wrote: >> On Fri, Oct 04, 2013 at 03:57:57PM +0200, Michal Simek wrote: >>> But anyway what was resolution from that meeting? >> >> It never happened, we got distracted by lunch :) Then why not to have it here? M -- 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