| Message ID | 1422111903-22176-1-git-send-email-robert.jarzmik@free.fr |
|---|---|
| State | Needs Review / ACK, archived |
| Headers | show |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 1 errors, 0 warnings, 0 lines checked |
| robh/patch-applied | success |
Robert Jarzmik <robert.jarzmik@free.fr> writes: > Add a binding for lubbock motherboard IO board. > > Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> > --- > Since v3: name change to lubbock-cplds, > Lee's comments taken into account. Hi Lee, I hope I have handled all the comments. Is this v4 good for you for mfd tree staging ? Cheers. -- Robert -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, 24 Jan 2015, Robert Jarzmik wrote: > Lubbock () board is the IO motherboard of the Intel PXA25x Development > Platform, which supports the Lubbock pxa25x soc board. > > Historically, this support was in arch/arm/mach-pxa/lubbock.c. When > gpio-pxa was moved to drivers/pxa, it became a driver, and its > initialization and probing happened at postcore initcall. The lubbock > code used to install the chained lubbock interrupt handler at init_irq() > time. > > The consequence of the gpio-pxa change is that the installed chained irq > handler lubbock_irq_handler() was overwritten in pxa_gpio_probe(_dt)(), > removing : > - the handler > - the falling edge detection setting of GPIO0, which revealed the > interrupt request from the lubbock IO board. > > As a fix, move the gpio0 chained handler setup to a place where we have > the guarantee that pxa_gpio_probe() was called before, so that lubbock > handler becomes the true IRQ chained handler of GPIO0, demuxing the > lubbock IO board interrupts. > > This patch moves all that handling to a mfd driver. It's only purpose > for the time being is the interrupt handling, but in the future it > should encompass all the motherboard CPLDs handling : > - leds > - switches > - hexleds > > Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> > --- > Since v1: change the name from cottula to lubbock_io > Dmitry pointed out the Cottula was the pxa25x family name, > lubbock was the pxa25x development board name. Therefore the > name was changed to lubbock_io (lubbock IO board) Are you sure this is what you want to do? We don't usually support 'boards' per say. Instead we support 'devices', then pull each of those devices together using some h/w description mechanism. Board files are so yesterday! Besides, this is MFD, where we support single pieces of silicon which happen to support multiple devices. I definitely don't want to support boards here. You might want to re-think the naming and your (sales) pitch. > change the resources to bi-irq ioresource > Discussion between Arnd and Robert to change the gpio > request by a irq request. > Since v2: take into account Mark's review > Use irq flags from resources (DT case and pdata case). > Change of name from lubbock_io to lubbock-io > Since v3: take into account Lee's review + Russell's advice > whitespace errors fixes > debug/info messages formatting > return X; empty lines > --- > drivers/mfd/Kconfig | 10 +++ > drivers/mfd/Makefile | 1 + > drivers/mfd/lubbock.c | 199 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 210 insertions(+) > create mode 100644 drivers/mfd/lubbock.c > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > index 2e6b731..4d8939f 100644 > --- a/drivers/mfd/Kconfig > +++ b/drivers/mfd/Kconfig > @@ -91,6 +91,16 @@ config MFD_AXP20X > components like regulators or the PEK (Power Enable Key) under the > corresponding menus. > > +config MFD_LUBBOCK > + bool "Lubbock Motherboard" > + def_bool ARCH_LUBBOCK > + select MFD_CORE > + help > + This driver supports the Lubbock multifunction chip found on the > + pxa25x development platform system (named Lubbock). This IO board > + supports the interrupts handling, ethernet controller, flash chips, > + etc ... > + > config MFD_CROS_EC > tristate "ChromeOS Embedded Controller" > select MFD_CORE > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > index 53467e2..aff1f4f 100644 > --- a/drivers/mfd/Makefile > +++ b/drivers/mfd/Makefile > @@ -9,6 +9,7 @@ obj-$(CONFIG_MFD_88PM805) += 88pm805.o 88pm80x.o > obj-$(CONFIG_MFD_SM501) += sm501.o > obj-$(CONFIG_MFD_ASIC3) += asic3.o tmio_core.o > obj-$(CONFIG_MFD_BCM590XX) += bcm590xx.o > +obj-$(CONFIG_MFD_LUBBOCK) += lubbock.o > obj-$(CONFIG_MFD_CROS_EC) += cros_ec.o > obj-$(CONFIG_MFD_CROS_EC_I2C) += cros_ec_i2c.o > obj-$(CONFIG_MFD_CROS_EC_SPI) += cros_ec_spi.o > diff --git a/drivers/mfd/lubbock.c b/drivers/mfd/lubbock.c > new file mode 100644 > index 0000000..4077fc5 > --- /dev/null > +++ b/drivers/mfd/lubbock.c > @@ -0,0 +1,199 @@ > +/* > + * Intel Cotulla MFD - lubbock motherboard > + * > + * Copyright (C) 2014 Robert Jarzmik > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * Lubbock motherboard driver, supporting Lubbock (aka. PXA25X) SoC board. > + */ > + > +#include <linux/bitops.h> > +#include <linux/gpio.h> > +#include <linux/gpio/consumer.h> > +#include <linux/interrupt.h> > +#include <linux/io.h> > +#include <linux/irq.h> > +#include <linux/irqdomain.h> > +#include <linux/mfd/core.h> Why have you included this? I don't see the use of the MFD framework anywhere. So what makes this an MFD? I'm going to stop here, as I think I need more of an explanation so what you're trying to achieve with this driver. [...]
----- Mail original ----- De: "Lee Jones" <lee.jones@linaro.org> À: "Robert Jarzmik" <robert.jarzmik@free.fr> Cc: "Rob Herring" <robh+dt@kernel.org>, "Pawel Moll" <pawel.moll@arm.com>, "Mark Rutland" <mark.rutland@arm.com>, "Ian Campbell" <ijc+devicetree@hellion.org.uk>, "Kumar Gala" <galak@codeaurora.org>, "Daniel Mack" <daniel@zonque.org>, "Haojian Zhuang" <haojian.zhuang@gmail.com>, "Samuel Ortiz" <sameo@linux.intel.com>, "Grant Likely" <grant.likely@linaro.org>, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, "Arnd Bergmann" <arnd@arndb.de>, "Russell King - ARM Linux" <linux@arm.linux.org.uk>, "Dmitry Eremin-Solenikov" <dbaryshkov@gmail.com> Envoyé: Lundi 16 Février 2015 14:05:49 Objet: Re: [PATCH v4 2/4] mfd: lubbock_cplds: add lubbock IO board On Sat, 24 Jan 2015, Robert Jarzmik wrote: > --- > Since v1: change the name from cottula to lubbock_io > Dmitry pointed out the Cottula was the pxa25x family name, > lubbock was the pxa25x development board name. Therefore the > name was changed to lubbock_io (lubbock IO board) > Are you sure this is what you want to do? We don't usually support > 'boards' per say. Instead we support 'devices', then pull each of > those devices together using some h/w description mechanism. Do you know that : 1) anything under "---" in a commit message is thrown away 2) after v2, we _both_ agreed that the accurate name is "cplds" which exactly what is in this patch (see device registering with lubbock_cplds). 3) there is no more mention of "board" anywhere in the patch core > Besides, this is MFD, where we support single pieces of silicon which > happen to support multiple devices. I definitely don't want to support > boards here. > You might want to re-think the naming and your (sales) pitch. I might need help. As for the (sales), no comment. >> +#include <linux/mfd/core.h> > Why have you included this? I don't see the use of the MFD framework > anywhere. So what makes this an MFD? I thought cplds were to be handled by an MFD driver. > I'm going to stop here, as I think I need more of an explanation so > what you're trying to achieve with this driver. Why ? I think things were clear that this driver handles the CPLDs on lubbock board, namely u46 and u52. I don't understand what is wrong with this patch so that you don't want to go forward. -- Robert -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 16 Feb 2015, robert.jarzmik@free.fr wrote: > ----- Mail original ----- > De: "Lee Jones" <lee.jones@linaro.org> > À: "Robert Jarzmik" <robert.jarzmik@free.fr> > Cc: "Rob Herring" <robh+dt@kernel.org>, "Pawel Moll" <pawel.moll@arm.com>, "Mark Rutland" <mark.rutland@arm.com>, "Ian Campbell" <ijc+devicetree@hellion.org.uk>, "Kumar Gala" <galak@codeaurora.org>, "Daniel Mack" <daniel@zonque.org>, "Haojian Zhuang" <haojian.zhuang@gmail.com>, "Samuel Ortiz" <sameo@linux.intel.com>, "Grant Likely" <grant.likely@linaro.org>, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, "Arnd Bergmann" <arnd@arndb.de>, "Russell King - ARM Linux" <linux@arm.linux.org.uk>, "Dmitry Eremin-Solenikov" <dbaryshkov@gmail.com> > Envoyé: Lundi 16 Février 2015 14:05:49 > Objet: Re: [PATCH v4 2/4] mfd: lubbock_cplds: add lubbock IO board What's all this? Please configure your mail client correctly. For advice, see: Documentation/email-clients.txt > On Sat, 24 Jan 2015, Robert Jarzmik wrote: > > > --- > > Since v1: change the name from cottula to lubbock_io > > Dmitry pointed out the Cottula was the pxa25x family name, > > lubbock was the pxa25x development board name. Therefore the > > name was changed to lubbock_io (lubbock IO board) > > > Are you sure this is what you want to do? We don't usually support > > 'boards' per say. Instead we support 'devices', then pull each of > > those devices together using some h/w description mechanism. > > Do you know that : > 1) anything under "---" in a commit message is thrown away Yes. But I don't remember reading this passage before. Just becuase this is no longer v2, it doesn't void any comments regarding v1 changelogs. > 2) after v2, we _both_ agreed that the accurate name is "cplds" > which exactly what is in this patch > (see device registering with lubbock_cplds). There is no mention of this decision in the changelogs. If it's not in the change logs, it didn't happen. ;) I'm still concerned with the fact that the driver file is named using and is populated by lots of instances of a "board" name. I'm sure you would share my thoughts is someone submitted a driver called beaglebone.c or raspberrypi.c to MFD. > 3) there is no more mention of "board" anywhere in the patch core No, just the name of one. > > Besides, this is MFD, where we support single pieces of silicon which > > happen to support multiple devices. I definitely don't want to support > > boards here. > > You might want to re-think the naming and your (sales) pitch. > I might need help. As for the (sales), no comment. By pitch, I mean to convince me that this belongs in MFD. > >> +#include <linux/mfd/core.h> > > Why have you included this? I don't see the use of the MFD framework > > anywhere. So what makes this an MFD? > I thought cplds were to be handled by an MFD driver. Not to my knowledge, although we do appear to have one. That doesn't necessarily mean that it's the right place for it though. I'm not entirely sure how CPLDs even work on a functional level. > > I'm going to stop here, as I think I need more of an explanation so > > what you're trying to achieve with this driver. > Why ? I think things were clear that this driver handles the CPLDs on > lubbock board, namely u46 and u52. I don't understand what is wrong > with this patch so that you don't want to go forward. Understanding was lost when I learned that the whole file was centred around the premise of board support. I understand now that this is a driver for a CPLD, which I'm not sure is even MFD. Also, if it is really a CPLD driver, shouldn't be named after the manufacturer/model number of the chip, rather than the PCB which it's connected to? I'm also concerned that this driver has no functional CPLD code contained. All you're doing is defining an IRQ domain. Why then isn't this really an drivers/irqchip (Suggested-by: Josh Cartwright) driver?
Lee Jones <lee.jones@linaro.org> writes: > What's all this? Please configure your mail client correctly. > > For advice, see: > > Documentation/email-clients.txt While at day work, I have only access to web mail ... >> 2) after v2, we _both_ agreed that the accurate name is "cplds" >> which exactly what is in this patch >> (see device registering with lubbock_cplds). > > There is no mention of this decision in the changelogs. If it's not > in the change logs, it didn't happen. ;) Ah right. > I'm still concerned with the fact that the driver file is named using > and is populated by lots of instances of a "board" name. I'm sure you > would share my thoughts is someone submitted a driver called > beaglebone.c or raspberrypi.c to MFD. I understand your concern. Arnd, a thought about it ? The only viable alternative would be to move it to arch/arm/plat-pxa AFAIS. >> > Besides, this is MFD, where we support single pieces of silicon which >> > happen to support multiple devices. I definitely don't want to support >> > boards here. >> > You might want to re-think the naming and your (sales) pitch. >> I might need help. As for the (sales), no comment. > > By pitch, I mean to convince me that this belongs in MFD. I've been trying. >> I thought cplds were to be handled by an MFD driver. > > Not to my knowledge, although we do appear to have one. That doesn't > necessarily mean that it's the right place for it though. I'm not > entirely sure how CPLDs even work on a functional level. As I said, I understand your concern. >> > I'm going to stop here, as I think I need more of an explanation so >> > what you're trying to achieve with this driver. >> Why ? I think things were clear that this driver handles the CPLDs on >> lubbock board, namely u46 and u52. I don't understand what is wrong >> with this patch so that you don't want to go forward. > > Understanding was lost when I learned that the whole file was centred > around the premise of board support. I understand now that this is a > driver for a CPLD, which I'm not sure is even MFD. Also, if it is > really a CPLD driver, shouldn't be named after the manufacturer/model > number of the chip, rather than the PCB which it's connected to? > > I'm also concerned that this driver has no functional CPLD code > contained. All you're doing is defining an IRQ domain. Why then > isn't this really an drivers/irqchip (Suggested-by: Josh Cartwright) > driver? Is not only a irqchip because, as explained at the bottom of the commit message, quoting myself : This patch moves all that handling to a mfd driver. It's only purpose for the time being is the interrupt handling, but in the future it should encompass all the motherboard CPLDs handling : - leds - switches - hexleds When these parts will be added, and they'll have to be handled by this driver because of iospace sharing, and same IP (cplds) providing the hardware support, the irqchip way will be just impossible to follow. As I said above, I understand your concern. If we all reach a concensus this should be moved to the pxa tree, so be it. But I'd like other opinion here. Cheers.
Arnd, Greg, Perhaps you have some ideas WRT programmables (PLDs/CPLDs/FPGAs)? On Mon, 16 Feb 2015, Robert Jarzmik wrote: > Lee Jones wrote: > > What's all this? Please configure your mail client correctly. > > > > For advice, see: > > > > Documentation/email-clients.txt > While at day work, I have only access to web mail ... Probably best to hold off your reply until you get home then. It's not just a formatting issue, you also exposed many raw email addresses to the internet, which are easily harvested up by web crawlers. See this: https://lkml.org/lkml/2015/2/16/247 > >> 2) after v2, we _both_ agreed that the accurate name is "cplds" > >> which exactly what is in this patch > >> (see device registering with lubbock_cplds). > > > > There is no mention of this decision in the changelogs. If it's not > > in the change logs, it didn't happen. ;) > Ah right. > > > I'm still concerned with the fact that the driver file is named using > > and is populated by lots of instances of a "board" name. I'm sure you > > would share my thoughts is someone submitted a driver called > > beaglebone.c or raspberrypi.c to MFD. > I understand your concern. Arnd, a thought about it ? The only viable > alternative would be to move it to arch/arm/plat-pxa AFAIS. I don't think this is correct either. CPLD handling would probably be slightly less out of place in drivers/misc, but perhaps a new subsystem for PLDs/CPLDs/FPGAs would be more appropriate drivers/programmables or similar maybe. > >> > Besides, this is MFD, where we support single pieces of silicon which > >> > happen to support multiple devices. I definitely don't want to support > >> > boards here. > >> > You might want to re-think the naming and your (sales) pitch. > >> I might need help. As for the (sales), no comment. > > > > By pitch, I mean to convince me that this belongs in MFD. > I've been trying. I'm pretty convinced that it doesn't belong in MFD now, but it doesn't mean I'm going to leave you on the curb. I'd like to help you get it into a better home. [...] > > Understanding was lost when I learned that the whole file was centred > > around the premise of board support. I understand now that this is a > > driver for a CPLD, which I'm not sure is even MFD. Also, if it is > > really a CPLD driver, shouldn't be named after the manufacturer/model > > number of the chip, rather than the PCB which it's connected to? > > > > I'm also concerned that this driver has no functional CPLD code > > contained. All you're doing is defining an IRQ domain. Why then > > isn't this really an drivers/irqchip (Suggested-by: Josh Cartwright) > > driver? > Is not only a irqchip because, as explained at the bottom of the commit message, > quoting myself : > This patch moves all that handling to a mfd driver. It's only purpose > for the time being is the interrupt handling, but in the future it > should encompass all the motherboard CPLDs handling : > - leds > - switches > - hexleds I had a conversation about this on IRC yesterday and some good points/questions were posed. This is a difficult area, because you can program these things to do whatever you like. Depending on the 'intention' (and it is only an intention -- someone else can come along and reprogram these devices on a whim), the CPLD code could live anywhere. If you wanted to put watchdog functionality in there, then there is an argument for it to live in drivers/watchdog, etc etc. So just because the plan is to support a few (i.e. more than one) simple devices, it doesn't necessarily mean that the handling should be done in MFD. Yesterday I was asked "Are you wanting to restrict drivers in drivers/mfd to those that make use of MFD_CORE functionality?". My answer to that was "No, however; I only want devices which _intrinsically_ operate in multiple subsystems", which these programmables no not do. FYI, you're not on your own here. There is at least one of these devices in the kernel already and upon a short inspection there appears to be a number of Out-of-Tree (OoT) drivers out there which will require a home in Mainline sooner or later.
On Tue, 17 Feb 2015, Lee Jones wrote: > Arnd, Greg, > > Perhaps you have some ideas WRT programmables (PLDs/CPLDs/FPGAs)? FWIW... The Lubbock is an ancient development board (circa 2003) using a CPLD to multiplex a couple things on the board. I really doubt anyone would reprogram this CPLD at this point. So I'd treat it just like another interrupt controller + random peripherals that will never change. And yes, maybe a more appropriate name is needed. And I happen to have one such beast on my desk wasting significant realestate and picking up dust. I don't think I booted it even once in the last 3 years. I'm seriously considering a trip to the recycling facility to dispose of it unless someone wants it and is ready to pay for the shipping. > On Mon, 16 Feb 2015, Robert Jarzmik wrote: > > Lee Jones wrote: > > > What's all this? Please configure your mail client correctly. > > > > > > For advice, see: > > > > > > Documentation/email-clients.txt > > While at day work, I have only access to web mail ... > > Probably best to hold off your reply until you get home then. It's > not just a formatting issue, you also exposed many raw email addresses > to the internet, which are easily harvested up by web crawlers. > > See this: > https://lkml.org/lkml/2015/2/16/247 > > > >> 2) after v2, we _both_ agreed that the accurate name is "cplds" > > >> which exactly what is in this patch > > >> (see device registering with lubbock_cplds). > > > > > > There is no mention of this decision in the changelogs. If it's not > > > in the change logs, it didn't happen. ;) > > Ah right. > > > > > I'm still concerned with the fact that the driver file is named using > > > and is populated by lots of instances of a "board" name. I'm sure you > > > would share my thoughts is someone submitted a driver called > > > beaglebone.c or raspberrypi.c to MFD. > > I understand your concern. Arnd, a thought about it ? The only viable > > alternative would be to move it to arch/arm/plat-pxa AFAIS. > > I don't think this is correct either. CPLD handling would probably be > slightly less out of place in drivers/misc, but perhaps a new > subsystem for PLDs/CPLDs/FPGAs would be more appropriate > drivers/programmables or similar maybe. > > > >> > Besides, this is MFD, where we support single pieces of silicon which > > >> > happen to support multiple devices. I definitely don't want to support > > >> > boards here. > > >> > You might want to re-think the naming and your (sales) pitch. > > >> I might need help. As for the (sales), no comment. > > > > > > By pitch, I mean to convince me that this belongs in MFD. > > I've been trying. > > I'm pretty convinced that it doesn't belong in MFD now, but it doesn't > mean I'm going to leave you on the curb. I'd like to help you get it > into a better home. > > [...] > > > > Understanding was lost when I learned that the whole file was centred > > > around the premise of board support. I understand now that this is a > > > driver for a CPLD, which I'm not sure is even MFD. Also, if it is > > > really a CPLD driver, shouldn't be named after the manufacturer/model > > > number of the chip, rather than the PCB which it's connected to? > > > > > > I'm also concerned that this driver has no functional CPLD code > > > contained. All you're doing is defining an IRQ domain. Why then > > > isn't this really an drivers/irqchip (Suggested-by: Josh Cartwright) > > > driver? > > Is not only a irqchip because, as explained at the bottom of the commit message, > > quoting myself : > > This patch moves all that handling to a mfd driver. It's only purpose > > for the time being is the interrupt handling, but in the future it > > should encompass all the motherboard CPLDs handling : > > - leds > > - switches > > - hexleds > > I had a conversation about this on IRC yesterday and some good > points/questions were posed. This is a difficult area, because you > can program these things to do whatever you like. Depending on the > 'intention' (and it is only an intention -- someone else can come > along and reprogram these devices on a whim), the CPLD code could live > anywhere. If you wanted to put watchdog functionality in there, then > there is an argument for it to live in drivers/watchdog, etc etc. So > just because the plan is to support a few (i.e. more than one) simple > devices, it doesn't necessarily mean that the handling should be done > in MFD. > > Yesterday I was asked "Are you wanting to restrict drivers in > drivers/mfd to those that make use of MFD_CORE functionality?". My > answer to that was "No, however; I only want devices which > _intrinsically_ operate in multiple subsystems", which these > programmables no not do. > > FYI, you're not on your own here. There is at least one of these > devices in the kernel already and upon a short inspection there > appears to be a number of Out-of-Tree (OoT) drivers out there which > will require a home in Mainline sooner or later. > > -- > Lee Jones > Linaro STMicroelectronics Landing Team Lead > Linaro.org │ Open source software for ARM SoCs > Follow Linaro: Facebook | Twitter | Blog > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > >
On Tue, 17 Feb 2015, Nicolas Pitre wrote: > On Tue, 17 Feb 2015, Lee Jones wrote: > > > Arnd, Greg, > > > > Perhaps you have some ideas WRT programmables (PLDs/CPLDs/FPGAs)? > > FWIW... > > The Lubbock is an ancient development board (circa 2003) using a CPLD to > multiplex a couple things on the board. I really doubt anyone would > reprogram this CPLD at this point. So I'd treat it just like another > interrupt controller + random peripherals that will never change. And > yes, maybe a more appropriate name is needed. Understood. However, I'm tempted to occupy some higher ground here and say that there will always be (good?) excuses to shoehorn drivers into subsystems which they don't truly belong. Rather than make exceptions I'd rather see an implementation which would also serve subsequent attempts to upstream these types of devices. > And I happen to have one such beast on my desk wasting significant > realestate and picking up dust. I don't think I booted it even once in > the last 3 years. I'm seriously considering a trip to the recycling > facility to dispose of it unless someone wants it and is ready to pay > for the shipping. Shameless. I think you were looking for Craig's List. ;) > > On Mon, 16 Feb 2015, Robert Jarzmik wrote: > > > Lee Jones wrote: > > > > What's all this? Please configure your mail client correctly. > > > > > > > > For advice, see: > > > > > > > > Documentation/email-clients.txt > > > While at day work, I have only access to web mail ... > > > > Probably best to hold off your reply until you get home then. It's > > not just a formatting issue, you also exposed many raw email addresses > > to the internet, which are easily harvested up by web crawlers. > > > > See this: > > https://lkml.org/lkml/2015/2/16/247 > > > > > >> 2) after v2, we _both_ agreed that the accurate name is "cplds" > > > >> which exactly what is in this patch > > > >> (see device registering with lubbock_cplds). > > > > > > > > There is no mention of this decision in the changelogs. If it's not > > > > in the change logs, it didn't happen. ;) > > > Ah right. > > > > > > > I'm still concerned with the fact that the driver file is named using > > > > and is populated by lots of instances of a "board" name. I'm sure you > > > > would share my thoughts is someone submitted a driver called > > > > beaglebone.c or raspberrypi.c to MFD. > > > I understand your concern. Arnd, a thought about it ? The only viable > > > alternative would be to move it to arch/arm/plat-pxa AFAIS. > > > > I don't think this is correct either. CPLD handling would probably be > > slightly less out of place in drivers/misc, but perhaps a new > > subsystem for PLDs/CPLDs/FPGAs would be more appropriate > > drivers/programmables or similar maybe. > > > > > >> > Besides, this is MFD, where we support single pieces of silicon which > > > >> > happen to support multiple devices. I definitely don't want to support > > > >> > boards here. > > > >> > You might want to re-think the naming and your (sales) pitch. > > > >> I might need help. As for the (sales), no comment. > > > > > > > > By pitch, I mean to convince me that this belongs in MFD. > > > I've been trying. > > > > I'm pretty convinced that it doesn't belong in MFD now, but it doesn't > > mean I'm going to leave you on the curb. I'd like to help you get it > > into a better home. > > > > [...] > > > > > > Understanding was lost when I learned that the whole file was centred > > > > around the premise of board support. I understand now that this is a > > > > driver for a CPLD, which I'm not sure is even MFD. Also, if it is > > > > really a CPLD driver, shouldn't be named after the manufacturer/model > > > > number of the chip, rather than the PCB which it's connected to? > > > > > > > > I'm also concerned that this driver has no functional CPLD code > > > > contained. All you're doing is defining an IRQ domain. Why then > > > > isn't this really an drivers/irqchip (Suggested-by: Josh Cartwright) > > > > driver? > > > Is not only a irqchip because, as explained at the bottom of the commit message, > > > quoting myself : > > > This patch moves all that handling to a mfd driver. It's only purpose > > > for the time being is the interrupt handling, but in the future it > > > should encompass all the motherboard CPLDs handling : > > > - leds > > > - switches > > > - hexleds > > > > I had a conversation about this on IRC yesterday and some good > > points/questions were posed. This is a difficult area, because you > > can program these things to do whatever you like. Depending on the > > 'intention' (and it is only an intention -- someone else can come > > along and reprogram these devices on a whim), the CPLD code could live > > anywhere. If you wanted to put watchdog functionality in there, then > > there is an argument for it to live in drivers/watchdog, etc etc. So > > just because the plan is to support a few (i.e. more than one) simple > > devices, it doesn't necessarily mean that the handling should be done > > in MFD. > > > > Yesterday I was asked "Are you wanting to restrict drivers in > > drivers/mfd to those that make use of MFD_CORE functionality?". My > > answer to that was "No, however; I only want devices which > > _intrinsically_ operate in multiple subsystems", which these > > programmables no not do. > > > > FYI, you're not on your own here. There is at least one of these > > devices in the kernel already and upon a short inspection there > > appears to be a number of Out-of-Tree (OoT) drivers out there which > > will require a home in Mainline sooner or later. > >
Robert Jarzmik <robert.jarzmik@free.fr> writes: > Hi Arnd and Greg, It's been a week, backlog ping ? > > I have this driver I'm upstreaming, which comes out of > arch/arm/mach-pxa/lubbock.c. As for the reason it is extracted, see submitted > commit [1] for reference. > > The main question is : where does it belong in the kernel ? > > The driver is : > - for the CPLDs on the Lubbock development platform, which is more or less an > old motherboard for Intel Xscale pxa255 SoC (see [2] for more details) > - these CPLDs control : > - interrupt muxing towards the SoC > - several leds > - switches read back > For the whole patch, see [4] > > Lee's position is that it doesn't belong to drivers/mfd, see [3]. > > So where should I submit it ? And more generally, where should CPLDs drivers be > pushed in the kernel tree ? > > If there is no solution, I'll fallback through arch/arm/plat-pxa, not very nice, > but it has to land somewhere, I don't want lubbock to remain broken. > > Cheers. > > -- > Robert > > [1] Reason of extraction / commit message > mfd: lubbock_cplds: add lubbock IO board > > Lubbock () board is the IO motherboard of the Intel PXA25x Development > Platform, which supports the Lubbock pxa25x soc board. > > Historically, this support was in arch/arm/mach-pxa/lubbock.c. When > gpio-pxa was moved to drivers/pxa, it became a driver, and its > initialization and probing happened at postcore initcall. The lubbock > code used to install the chained lubbock interrupt handler at init_irq() > time. > > The consequence of the gpio-pxa change is that the installed chained irq > handler lubbock_irq_handler() was overwritten in pxa_gpio_probe(_dt)(), > removing : > - the handler > - the falling edge detection setting of GPIO0, which revealed the > interrupt request from the lubbock IO board. > > As a fix, move the gpio0 chained handler setup to a place where we have > the guarantee that pxa_gpio_probe() was called before, so that lubbock > handler becomes the true IRQ chained handler of GPIO0, demuxing the > lubbock IO board interrupts. > > This patch moves all that handling to a mfd driver. It's only purpose > for the time being is the interrupt handling, but in the future it > should encompass all the motherboard CPLDs handling : > - leds > - switches > - hexleds > > Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> > > [2] Board description by Nicolas >>> The Lubbock is an ancient development board (circa 2003) using a CPLD to >>> multiplex a couple things on the board. I really doubt anyone would >>> reprogram this CPLD at this point. So I'd treat it just like another >>> interrupt controller + random peripherals that will never change. And >>> yes, maybe a more appropriate name is needed. > > [3] Lee's position >>> > I don't think this is correct either. CPLD handling would probably be >>> > slightly less out of place in drivers/misc, but perhaps a new >>> > subsystem for PLDs/CPLDs/FPGAs would be more appropriate >>> > drivers/programmables or similar maybe. >>> > > ... >>> > I'm pretty convinced that it doesn't belong in MFD now, but it doesn't >>> > mean I'm going to leave you on the curb. I'd like to help you get it >>> > into a better home. >>> > >>> > [...] >>> > > Is not only a irqchip because, as explained at the bottom of the commit message, >>> > > quoting myself : >>> > > This patch moves all that handling to a mfd driver. It's only purpose >>> > > for the time being is the interrupt handling, but in the future it >>> > > should encompass all the motherboard CPLDs handling : >>> > > - leds >>> > > - switches >>> > > - hexleds >>> > >>> > I had a conversation about this on IRC yesterday and some good >>> > points/questions were posed. This is a difficult area, because you >>> > can program these things to do whatever you like. Depending on the >>> > 'intention' (and it is only an intention -- someone else can come >>> > along and reprogram these devices on a whim), the CPLD code could live >>> > anywhere. If you wanted to put watchdog functionality in there, then >>> > there is an argument for it to live in drivers/watchdog, etc etc. So >>> > just because the plan is to support a few (i.e. more than one) simple >>> > devices, it doesn't necessarily mean that the handling should be done >>> > in MFD. >>> > >>> > Yesterday I was asked "Are you wanting to restrict drivers in >>> > drivers/mfd to those that make use of MFD_CORE functionality?". My >>> > answer to that was "No, however; I only want devices which >>> > _intrinsically_ operate in multiple subsystems", which these >>> > programmables no not do. >>> > >>> > FYI, you're not on your own here. There is at least one of these >>> > devices in the kernel already and upon a short inspection there >>> > appears to be a number of Out-of-Tree (OoT) drivers out there which >>> > will require a home in Mainline sooner or later. >>> > > > [4] Whole patch > https://lkml.org/lkml/2015/1/24/90
On Sat, Feb 28, 2015 at 10:57:30AM +0100, Robert Jarzmik wrote: > Robert Jarzmik <robert.jarzmik@free.fr> writes: > > > Hi Arnd and Greg, > It's been a week, backlog ping ? If only my backlog was just one week... And I'm not the mfd maintainer... -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes: > On Sat, Feb 28, 2015 at 10:57:30AM +0100, Robert Jarzmik wrote: >> Robert Jarzmik <robert.jarzmik@free.fr> writes: >> >> > Hi Arnd and Greg, >> It's been a week, backlog ping ? > > If only my backlog was just one week... Ah, that's bad :) > And I'm not the mfd maintainer... I know. The question was for the "all drivers" maintainer ... ie. you. The question was where should CPLD drivers end up, given that the mfd maintainer pushed back. Yet I understand you're overflowed, and I'll take it into arch/arm/plat-pxa. I don't think it's the right place, but it's my only fallback. Cheers.
On Fri, Feb 20, 2015 at 05:02:57PM +0100, Robert Jarzmik wrote: > Lee Jones <lee.jones@linaro.org> writes: > >> > Arnd, Greg, > >> > > >> > Perhaps you have some ideas WRT programmables (PLDs/CPLDs/FPGAs)? > > Hi Arnd and Greg, > > I have this driver I'm upstreaming, which comes out of > arch/arm/mach-pxa/lubbock.c. As for the reason it is extracted, see submitted > commit [1] for reference. > > The main question is : where does it belong in the kernel ? > > The driver is : > - for the CPLDs on the Lubbock development platform, which is more or less an > old motherboard for Intel Xscale pxa255 SoC (see [2] for more details) > - these CPLDs control : > - interrupt muxing towards the SoC > - several leds > - switches read back > For the whole patch, see [4] > > Lee's position is that it doesn't belong to drivers/mfd, see [3]. > > So where should I submit it ? And more generally, where should CPLDs drivers be > pushed in the kernel tree ? > > If there is no solution, I'll fallback through arch/arm/plat-pxa, not very nice, > but it has to land somewhere, I don't want lubbock to remain broken. drivers/platform/arm ? -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes: > On Fri, Feb 20, 2015 at 05:02:57PM +0100, Robert Jarzmik wrote: >> If there is no solution, I'll fallback through arch/arm/plat-pxa, not very nice, >> but it has to land somewhere, I don't want lubbock to remain broken. > > drivers/platform/arm ? Most certainly. I'll submit that to drivers/platform/arm/pxa, and maintain that pxa tree. As for drivers/platform/arm, do you want also maintainers to step up, or will you take the review/merge burden ? Cheers.
On Thu, Mar 26, 2015 at 10:38:54PM +0100, Robert Jarzmik wrote: > Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes: > > > On Fri, Feb 20, 2015 at 05:02:57PM +0100, Robert Jarzmik wrote: > >> If there is no solution, I'll fallback through arch/arm/plat-pxa, not very nice, > >> but it has to land somewhere, I don't want lubbock to remain broken. > > > > drivers/platform/arm ? > Most certainly. > > I'll submit that to drivers/platform/arm/pxa, and maintain that pxa tree. As for > drivers/platform/arm, do you want also maintainers to step up, or will you take > the review/merge burden ? I have enough review/merge burden to do anything with ARM, sorry. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday 26 March 2015, Robert Jarzmik wrote: > > Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes: > > > On Fri, Feb 20, 2015 at 05:02:57PM +0100, Robert Jarzmik wrote: > >> If there is no solution, I'll fallback through arch/arm/plat-pxa, not very nice, > >> but it has to land somewhere, I don't want lubbock to remain broken. > > > > drivers/platform/arm ? > Most certainly. > > I'll submit that to drivers/platform/arm/pxa, and maintain that pxa tree. As for > drivers/platform/arm, do you want also maintainers to step up, or will you take > the review/merge burden ? > I'd much prefer not to add drivers/platform/arm, which would make it too easy to add random stuff there. What is the problem with leaving it in mach-pxa? Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Arnd Bergmann <arnd@arndb.de> writes: > On Thursday 26 March 2015, Robert Jarzmik wrote: >> >> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes: >> >> > On Fri, Feb 20, 2015 at 05:02:57PM +0100, Robert Jarzmik wrote: >> >> If there is no solution, I'll fallback through arch/arm/plat-pxa, not very nice, >> >> but it has to land somewhere, I don't want lubbock to remain broken. >> > >> > drivers/platform/arm ? >> Most certainly. >> >> I'll submit that to drivers/platform/arm/pxa, and maintain that pxa tree. As for >> drivers/platform/arm, do you want also maintainers to step up, or will you take >> the review/merge burden ? >> > > I'd much prefer not to add drivers/platform/arm, which would make it too easy > to add random stuff there. What is the problem with leaving it in mach-pxa? Hi Arnd, It's not as much a problem as a generic question : does a driver belong to arch/* ? Personaly it would have been far simpler for me to have it through the pxa tree, but I want to be sure it's the right place. Others will follow, pxa mainstone is such a candidate. I was thinking so far that arch/arm/mach-* was for machine description, ie. wirings, interconnections, initial setup etc ... The "driver" part, ie. code really driving dynamics in IPs was as per my understanding in drivers/... Now I can create arch/arm/mach-pxa/lubbock_cplds.c, that won't make any difference to me, provided that it's the right thing to do. Cheers.
On Saturday 28 March 2015, Robert Jarzmik wrote: > It's not as much a problem as a generic question : does a driver belong to > arch/* ? > > Personaly it would have been far simpler for me to have it through the pxa tree, > but I want to be sure it's the right place. Others will follow, pxa mainstone is > such a candidate. > > I was thinking so far that arch/arm/mach-* was for machine description, > ie. wirings, interconnections, initial setup etc ... The "driver" part, ie. code > really driving dynamics in IPs was as per my understanding in drivers/... > > Now I can create arch/arm/mach-pxa/lubbock_cplds.c, that won't make any > difference to me, provided that it's the right thing to do. If we had a lot of these, we would probably put them somewhere under drivers. and find a maintainer for them. Given that this is an exceptional case for an older machine, my feeling is that leaving the code in mach-pxa is the least effort for now. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/mfd/lubbock-cplds.txt b/Documentation/devicetree/bindings/mfd/lubbock-cplds.txt new file mode 100644 index 0000000..211ed9a --- /dev/null +++ b/Documentation/devicetree/bindings/mfd/lubbock-cplds.txt @@ -0,0 +1,26 @@ +Intel XScale PXA255 development platform (Lubbock). + +This regroups all the CPLDs on the Lubbock motherboard, providing interrupt +muxing, leds handling, ... + +Required properties: + - compatible : should be "intel,lubbock-cplds" + + - interrupts : The first interrupt is the SoC input interrupt connected to the + lubbock IO board interrupt multiplexer output. The only known + working conifguration is GPIO0 on the PXA25X SoC. + +Optional properties: + - interrupts : The second optional interrupt is the base of the interrupts + multiplexed by the lubbock motherboard. If unspecified, the + range is fully dynamic, and the irqdomain will generate its + interrupt base on the fly. + +Example: + +mb: lubbock-mb@0 { + compatible = "intel,lubbock-cplds"; + interrupts = <0 IRQ_TYPE_EDGE_FALLING 400 IRQ_TYPE_NONE>; + #interrupt-cells = <2>; + interrupt-parent = <&pxairq>; +};
Add a binding for lubbock motherboard IO board. Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> --- Since v3: name change to lubbock-cplds, Lee's comments taken into account. --- .../devicetree/bindings/mfd/lubbock-cplds.txt | 26 ++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 Documentation/devicetree/bindings/mfd/lubbock-cplds.txt