Re: [PATCH v4 2/4] mfd: lubbock_cplds: add lubbock IO board

Nicolas Pitre
2015-02-17T17:38:11Z

[-- Attachment #1: Type: TEXT/PLAIN, Size: 5450 bytes --]

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/
>
>