linux-kernel - Re: [PATCH v3] serial: 8250: add gpio support to exar

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20151222042707.GA3250@sudip-pc>
Date:	Tue, 22 Dec 2015 09:57:07 +0530
From:	Sudip Mukherjee <sudipm.mukherjee@...il.com>
To:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:	One Thousand Gnomes <gnomes@...rguk.ukuu.org.uk>,
	Andy Shevchenko <andy.shevchenko@...il.com>,
	Jiri Slaby <jslaby@...e.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>
Subject: Re: [PATCH v3] serial: 8250: add gpio support to exar

On Mon, Dec 21, 2015 at 08:28:51PM +0200, Andy Shevchenko wrote:
> On Mon, Dec 21, 2015 at 5:19 PM, Sudip Mukherjee
> <sudipm.mukherjee@...il.com> wrote:
> 
> > diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> > index 6412f14..ec3d287 100644
> > --- a/drivers/tty/serial/8250/Kconfig
> > +++ b/drivers/tty/serial/8250/Kconfig
> > @@ -378,3 +378,11 @@ config SERIAL_8250_MID
> >           Selecting this option will enable handling of the extra features
> >           present on the UART found on Intel Medfield SOC and various other
> >           Intel platforms.
> > +
> > +config SERIAL_8250_EXAR_GPIO
> > +       bool "Support for GPIO pins on XR17V352/354/358"
> > +       depends on SERIAL_8250_PCI && GPIOLIB
> > +       select GPIO_SYSFS
> > +       help
> > +         Selecting this option will enable handling of GPIO pins present
> > +         on Exar XR17V352/354/358 chips.
> > diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile
> > index e177f86..1d0fce0 100644
> > --- a/drivers/tty/serial/8250/Makefile
> > +++ b/drivers/tty/serial/8250/Makefile
> > @@ -28,5 +28,8 @@ obj-$(CONFIG_SERIAL_8250_MT6577)      += 8250_mtk.o
> >  obj-$(CONFIG_SERIAL_8250_UNIPHIER)     += 8250_uniphier.o
> >  obj-$(CONFIG_SERIAL_8250_INGENIC)      += 8250_ingenic.o
> >  obj-$(CONFIG_SERIAL_8250_MID)          += 8250_mid.o
> > +obj-$(CONFIG_SERIAL_8250_EXAR_GPIO)    += exar_gpio.o
> > +exar_gpio-y                            := 8250_pci.o 8250_gpio.o
> > +
> >
> >  CFLAGS_8250_ingenic.o += -I$(srctree)/scripts/dtc/libfdt
> >
> >
> > But I am getting:
> > ERROR: "xr17v35x_gpio_exit" [drivers/tty/serial/8250/8250_pci.ko] undefined!
> > ERROR: "xr17v35x_gpio_init" [drivers/tty/serial/8250/8250_pci.ko] undefined!
> >
> > I will need little hint for the Makefile, please. Its clear that i have
> > messed up.
> 
> Yes, read my previous mail.
> 
> There are at least two approaches:
>  - use 8250_pci, etc as a library (see example: 8250_mid.c)
>  - force 8250_pci to use external libraries in some cases (seems your approach)
> 
> In the latter you have to extend 8250_pci and rename it. I have no
> idea if Greg or anyone else gives a green light for that.

I am sure Greg will give a big red light for this. :)

> 
> Something like this (forget syntax of ifs in Makefile though
> obj-(…8250_PCI) += 8250_pci.o
> obj-8250_pci = 8250_pci_orig.o
> ifneq (…EXAR_GPIO,)
> obj-8250_pci += exar_gpio.o
> endif
> 
> But what I can recommend is to split exar parts out of 8250_pci first
> to a separate driver, do your part as a part of that new driver.

Ok, same approach what 8250_mid.c has done. But in the initial design
discussions Greg was against the idea of having it as a separate driver.

> Of course better to gather maintainer's opinion first.

Greg, can you please give some idea here about the best way to approach...
I personally think, having it as a module with the minor changes that
Alan and Andy has suggested is the best approach. The only downside is
that the module gets loaded even if the device is not there. But for
arguments sake, the module will be enabled in the config only if someone
is interested to build it. Everyone has the option to disable it if
required.

regards
sudip
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/