* [PATCH 0/2] gpio-f7188x: Add F81886 gpio function @ 2016-01-12 7:41 Peter Hung 2016-01-12 7:41 ` [PATCH 1/2] gpio-f7188x: add Fintek F81866 SuperIO support Peter Hung 2016-01-12 7:41 ` [PATCH 2/2] gpio-f7188x: filter non-export gpio for F81866 Peter Hung 0 siblings, 2 replies; 7+ messages in thread From: Peter Hung @ 2016-01-12 7:41 UTC (permalink / raw) To: linus.walleij, gnurou Cc: linux-gpio, linux-kernel, tom_tsai, peter_hong, Peter Hung Fintek F81886 SuperIO spec: http://www.alldatasheet.com/datasheet-pdf/pdf/459085/FINTEK/F81866AD-I.html The detail gpio check & configuration could be found within spec P126. It controls gpio is the same with F7188x and has max 9 set of gpios. The gpio address is below: GPIO0x based: 0xf0 GPIO1x based: 0xe0 GPIO2x based: 0xd0 GPIO3x based: 0xc0 GPIO4x based: 0xb0 GPIO5x based: 0xa0 GPIO6x based: 0x90 GPIO7x based: 0x80 GPIO8x based: 0x88 <-- not 0x70. The first patch only add basic structure for F81866 to use all unverified gpio, and second patch add filter function to make sure only enabled gpios are exported. Peter Hung (2): gpio-f7188x: add Fintek F81866 SuperIO support gpio-f7188x: filter non-export gpio for F81866 drivers/gpio/gpio-f7188x.c | 176 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 175 insertions(+), 1 deletion(-) -- 1.9.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] gpio-f7188x: add Fintek F81866 SuperIO support 2016-01-12 7:41 [PATCH 0/2] gpio-f7188x: Add F81886 gpio function Peter Hung @ 2016-01-12 7:41 ` Peter Hung 2016-01-12 7:41 ` [PATCH 2/2] gpio-f7188x: filter non-export gpio for F81866 Peter Hung 1 sibling, 0 replies; 7+ messages in thread From: Peter Hung @ 2016-01-12 7:41 UTC (permalink / raw) To: linus.walleij, gnurou Cc: linux-gpio, linux-kernel, tom_tsai, peter_hong, Peter Hung add basic F81866 SuperIO gpio support Signed-off-by: Peter Hung <hpeter+linux_kernel@gmail.com> --- drivers/gpio/gpio-f7188x.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/gpio-f7188x.c b/drivers/gpio/gpio-f7188x.c index 5e3c4fa..a6a9641 100644 --- a/drivers/gpio/gpio-f7188x.c +++ b/drivers/gpio/gpio-f7188x.c @@ -36,14 +36,16 @@ #define SIO_F71869A_ID 0x1007 /* F71869A chipset ID */ #define SIO_F71882_ID 0x0541 /* F71882 chipset ID */ #define SIO_F71889_ID 0x0909 /* F71889 chipset ID */ +#define SIO_F81866_ID 0x1010 /* F81866 chipset ID */ -enum chips { f71869, f71869a, f71882fg, f71889f }; +enum chips { f71869, f71869a, f71882fg, f71889f, f81866 }; static const char * const f7188x_names[] = { "f71869", "f71869a", "f71882fg", "f71889f", + "f81866", }; struct f7188x_sio { @@ -190,6 +192,18 @@ static struct f7188x_gpio_bank f71889_gpio_bank[] = { F7188X_GPIO_BANK(70, 8, 0x80), }; +static struct f7188x_gpio_bank f81866_gpio_bank[] = { + F7188X_GPIO_BANK(0, 8, 0xF0), + F7188X_GPIO_BANK(10, 8, 0xE0), + F7188X_GPIO_BANK(20, 8, 0xD0), + F7188X_GPIO_BANK(30, 8, 0xC0), + F7188X_GPIO_BANK(40, 8, 0xB0), + F7188X_GPIO_BANK(50, 8, 0xA0), + F7188X_GPIO_BANK(60, 8, 0x90), + F7188X_GPIO_BANK(70, 8, 0x80), + F7188X_GPIO_BANK(80, 8, 0x88), +}; + static int f7188x_gpio_direction_in(struct gpio_chip *chip, unsigned offset) { int err; @@ -322,6 +336,10 @@ static int f7188x_gpio_probe(struct platform_device *pdev) data->nr_bank = ARRAY_SIZE(f71889_gpio_bank); data->bank = f71889_gpio_bank; break; + case f81866: + data->nr_bank = ARRAY_SIZE(f81866_gpio_bank); + data->bank = f81866_gpio_bank; + break; default: return -ENODEV; } @@ -399,6 +417,9 @@ static int __init f7188x_find(int addr, struct f7188x_sio *sio) case SIO_F71889_ID: sio->type = f71889f; break; + case SIO_F81866_ID: + sio->type = f81866; + break; default: pr_info(DRVNAME ": Unsupported Fintek device 0x%04x\n", devid); goto err; -- 1.9.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] gpio-f7188x: filter non-export gpio for F81866 2016-01-12 7:41 [PATCH 0/2] gpio-f7188x: Add F81886 gpio function Peter Hung 2016-01-12 7:41 ` [PATCH 1/2] gpio-f7188x: add Fintek F81866 SuperIO support Peter Hung @ 2016-01-12 7:41 ` Peter Hung 2016-01-12 9:33 ` Andy Shevchenko 2016-01-12 15:36 ` Simon Guinot 1 sibling, 2 replies; 7+ messages in thread From: Peter Hung @ 2016-01-12 7:41 UTC (permalink / raw) To: linus.walleij, gnurou Cc: linux-gpio, linux-kernel, tom_tsai, peter_hong, Peter Hung Dont export gpios which not enabled by motherboard manufacturer. Signed-off-by: Peter Hung <hpeter+linux_kernel@gmail.com> --- drivers/gpio/gpio-f7188x.c | 153 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 153 insertions(+) diff --git a/drivers/gpio/gpio-f7188x.c b/drivers/gpio/gpio-f7188x.c index a6a9641..f7fe7aa 100644 --- a/drivers/gpio/gpio-f7188x.c +++ b/drivers/gpio/gpio-f7188x.c @@ -38,6 +38,12 @@ #define SIO_F71889_ID 0x0909 /* F71889 chipset ID */ #define SIO_F81866_ID 0x1010 /* F81866 chipset ID */ +#define F81866_PORT_SEL_REG 0x27 +#define F81866_MULTI_FUN1_REG 0x28 +#define F81866_MULTI_FUN3_REG 0x29 +#define F81866_MULTI_FUN4_REG 0x2B +#define F81866_GPIO_EN_REG 0x2C + enum chips { f71869, f71869a, f71882fg, f71889f, f81866 }; static const char * const f7188x_names[] = { @@ -93,6 +99,15 @@ static inline void superio_outb(int base, int reg, int val) outb(val, base + 1); } +static inline void superio_mask_outb(int base, int reg, int mask, int val) +{ + u8 tmp; + + tmp = superio_inb(base, reg); + tmp = (tmp & ~mask) | (val & mask); + superio_outb(base, reg, tmp); +} + static inline int superio_enter(int base) { /* Don't step on other drivers' I/O space by accident. */ @@ -304,6 +319,125 @@ static void f7188x_gpio_set(struct gpio_chip *chip, unsigned offset, int value) superio_exit(sio->addr); } +static int f81866_verify_gpioset(int base, int set) +{ + int err; + u8 tmp; + + err = superio_enter(base); + if (err) + return err; + + err = -ENODEV; + + switch (set) { + case 0: + superio_mask_outb(base, F81866_PORT_SEL_REG, BIT(3) | BIT(2) | + BIT(0), 0); + tmp = superio_inb(base, F81866_GPIO_EN_REG); + if ((tmp & 0x1f) != 0x1f) + break; /* one in GPIO00~GPIO04 is not enable */ + + tmp = superio_inb(base, F81866_MULTI_FUN1_REG); + if ((tmp & 0xc) != 0x0) + break; + + err = 0; /* GPIO0x set is all enabled */ + break; + case 1: + superio_mask_outb(base, F81866_PORT_SEL_REG, BIT(3) | BIT(2) | + BIT(0), BIT(2)); + tmp = superio_inb(base, F81866_GPIO_EN_REG); + if ((tmp & 0xef) != 0xef) + break; /* one in GPIO10~GPIO17 is not enable */ + + tmp = superio_inb(base, F81866_MULTI_FUN3_REG); + if ((tmp & 0x03) != 0x00) + break; /* one in GPIO12~GPIO13 is not enable */ + + err = 0; /* GPIO1x set is all enabled */ + break; + case 2: + superio_mask_outb(base, F81866_PORT_SEL_REG, BIT(3) | BIT(2) | + BIT(0), BIT(3)); + tmp = superio_inb(base, F81866_GPIO_EN_REG); + if ((tmp & 0xff) != 0xff) + break; /* one in GPIO20~GPIO27 is not enable */ + + tmp = superio_inb(base, F81866_MULTI_FUN3_REG); + if ((tmp & 0x08) != 0x00) + break; /* GPIO20 is not enable */ + + err = 0; /* GPIO2x set is all enabled */ + break; + case 3: + superio_mask_outb(base, F81866_PORT_SEL_REG, BIT(0), 0); + tmp = superio_inb(base, F81866_MULTI_FUN3_REG); + if ((tmp & 0x30) != 0x00) + break; /* GPIO3x is not enable */ + + err = 0; /* GPIO3x set is all enabled */ + break; + case 4: + superio_mask_outb(base, F81866_PORT_SEL_REG, BIT(0), 0); + tmp = superio_inb(base, F81866_MULTI_FUN3_REG); + if ((tmp & 0xc0) != 0x00) + break; /* GPIO4x is not enable */ + + err = 0; /* GPIO4x set is all enabled */ + break; + case 5: + superio_mask_outb(base, F81866_PORT_SEL_REG, BIT(3) | BIT(2), + 0); + tmp = superio_inb(base, F81866_MULTI_FUN1_REG); + if ((tmp & 0x43) != 0x40) + break; /* GPIO5x is not enable */ + + err = 0; /* GPIO5x set is all enabled */ + break; + case 6: + superio_mask_outb(base, F81866_PORT_SEL_REG, BIT(3) | BIT(2) | + BIT(0), 0); + tmp = superio_inb(base, F81866_MULTI_FUN1_REG); + if ((tmp & 0x4c) != 0x40) + break; /* GPIO60~64 is not enable */ + + tmp = superio_inb(base, F81866_MULTI_FUN4_REG); + if ((tmp & 0xe0) != 0xe0) + break; /* GPIO65~67 is not enable */ + + err = 0; /* GPIO6x set is all enabled */ + break; + case 7: + superio_mask_outb(base, F81866_PORT_SEL_REG, BIT(3) | BIT(2) | + BIT(0), 0); + tmp = superio_inb(base, F81866_MULTI_FUN1_REG); + if ((tmp & 0x20) != 0x20) + break; /* GPIO7x is not enable */ + + tmp = superio_inb(base, F81866_MULTI_FUN4_REG); + if ((tmp & 0x01) != 0x00) + break; /* GPIO70 is not enable */ + + err = 0; /* GPIO7x set is all enabled */ + break; + case 8: + superio_mask_outb(base, F81866_PORT_SEL_REG, BIT(3) | BIT(2), + 0); + tmp = superio_inb(base, F81866_MULTI_FUN1_REG); + if ((tmp & 0x20) != 0x20) + break; /* GPIO8x is not enable */ + + err = 0; /* GPIO8x set is all enabled */ + break; + default: + break; + } + + superio_exit(base); + return err; +} + /* * Platform device and driver. */ @@ -351,6 +485,15 @@ static int f7188x_gpio_probe(struct platform_device *pdev) for (i = 0; i < data->nr_bank; i++) { struct f7188x_gpio_bank *bank = &data->bank[i]; + /* + * Dont export GPIO sysfs if pin set is not enable by MB + * manufacturer. + */ + if (sio->type == f81866 && f81866_verify_gpioset(sio->addr, i)) + continue; + + dev_dbg(&pdev->dev, "%s: register GPIO%xx set\n", __func__, + bank->chip.base >> 4); bank->chip.dev = &pdev->dev; bank->data = data; @@ -368,6 +511,11 @@ static int f7188x_gpio_probe(struct platform_device *pdev) err_gpiochip: for (i = i - 1; i >= 0; i--) { struct f7188x_gpio_bank *bank = &data->bank[i]; + + /* Some GPIO is not export, not need to remove */ + if (!bank->data || !bank->chip.dev) + continue; + gpiochip_remove(&bank->chip); } @@ -381,6 +529,11 @@ static int f7188x_gpio_remove(struct platform_device *pdev) for (i = 0; i < data->nr_bank; i++) { struct f7188x_gpio_bank *bank = &data->bank[i]; + + /* Some GPIO is not export, not need to remove */ + if (!bank->data || !bank->chip.dev) + continue; + gpiochip_remove(&bank->chip); } -- 1.9.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] gpio-f7188x: filter non-export gpio for F81866 2016-01-12 7:41 ` [PATCH 2/2] gpio-f7188x: filter non-export gpio for F81866 Peter Hung @ 2016-01-12 9:33 ` Andy Shevchenko 2016-01-13 2:03 ` Peter Hung 2016-01-12 15:36 ` Simon Guinot 1 sibling, 1 reply; 7+ messages in thread From: Andy Shevchenko @ 2016-01-12 9:33 UTC (permalink / raw) To: Peter Hung Cc: Linus Walleij, Alexandre Courbot, linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org, tom_tsai, Peter H, Peter Hung On Tue, Jan 12, 2016 at 9:41 AM, Peter Hung <hpeter@gmail.com> wrote: > Dont export gpios which not enabled by motherboard manufacturer. 'Do not' or 'Don't'. Perhaps 'GPIOs'. 'which are not enabled' > +#define F81866_PORT_SEL_REG 0x27 > +#define F81866_MULTI_FUN1_REG 0x28 > +#define F81866_MULTI_FUN3_REG 0x29 > +#define F81866_MULTI_FUN4_REG 0x2B > +#define F81866_GPIO_EN_REG 0x2C > + Move this before IDs block. Use same prefix SIO_ and drop _REG, e.g. SIO_F81866_PORT_SEL FUN -> FN or FUNC (Personally I prefer shorter) > enum chips { f71869, f71869a, f71882fg, f71889f, f81866 }; > > static const char * const f7188x_names[] = { > @@ -93,6 +99,15 @@ static inline void superio_outb(int base, int reg, int val) > outb(val, base + 1); > } > > +static inline void superio_mask_outb(int base, int reg, int mask, int val) Usually we named such as _update or _update_bits. superio_update() > +{ > + u8 tmp; > + > + tmp = superio_inb(base, reg); > + tmp = (tmp & ~mask) | (val & mask); > + superio_outb(base, reg, tmp); > +} > + > static inline int superio_enter(int base) > { > /* Don't step on other drivers' I/O space by accident. */ > @@ -304,6 +319,125 @@ static void f7188x_gpio_set(struct gpio_chip *chip, unsigned offset, int value) > superio_exit(sio->addr); > } > > +static int f81866_verify_gpioset(int base, int set) …_gpio_verify() looks more suitable to the existing scheme. > +{ > + int err; > + u8 tmp; > + > + err = superio_enter(base); > + if (err) > + return err; > + > + err = -ENODEV; > + > + switch (set) { > + case 0: > + superio_mask_outb(base, F81866_PORT_SEL_REG, BIT(3) | BIT(2) | > + BIT(0), 0); > + tmp = superio_inb(base, F81866_GPIO_EN_REG); > + if ((tmp & 0x1f) != 0x1f) Magic number. > + break; /* one in GPIO00~GPIO04 is not enable */ > + > + tmp = superio_inb(base, F81866_MULTI_FUN1_REG); > + if ((tmp & 0xc) != 0x0) Magic and no need to compare to != 0 explicitly. > + break; > + > + err = 0; /* GPIO0x set is all enabled */ Maybe you can refactor this in better way. Assign set of values in switch-case and do check after, only once. Same comments are applicable to other cases below. > + break; > + case 1: > + superio_mask_outb(base, F81866_PORT_SEL_REG, BIT(3) | BIT(2) | > + BIT(0), BIT(2)); > + tmp = superio_inb(base, F81866_GPIO_EN_REG); > + if ((tmp & 0xef) != 0xef) > + break; /* one in GPIO10~GPIO17 is not enable */ > + > + tmp = superio_inb(base, F81866_MULTI_FUN3_REG); > + if ((tmp & 0x03) != 0x00) > + break; /* one in GPIO12~GPIO13 is not enable */ > + > + err = 0; /* GPIO1x set is all enabled */ > + break; > + case 2: > + superio_mask_outb(base, F81866_PORT_SEL_REG, BIT(3) | BIT(2) | > + BIT(0), BIT(3)); > + tmp = superio_inb(base, F81866_GPIO_EN_REG); > + if ((tmp & 0xff) != 0xff) > + break; /* one in GPIO20~GPIO27 is not enable */ > + > + tmp = superio_inb(base, F81866_MULTI_FUN3_REG); > + if ((tmp & 0x08) != 0x00) > + break; /* GPIO20 is not enable */ > + > + err = 0; /* GPIO2x set is all enabled */ > + break; > + case 3: > + superio_mask_outb(base, F81866_PORT_SEL_REG, BIT(0), 0); > + tmp = superio_inb(base, F81866_MULTI_FUN3_REG); > + if ((tmp & 0x30) != 0x00) > + break; /* GPIO3x is not enable */ > + > + err = 0; /* GPIO3x set is all enabled */ > + break; > + case 4: > + superio_mask_outb(base, F81866_PORT_SEL_REG, BIT(0), 0); > + tmp = superio_inb(base, F81866_MULTI_FUN3_REG); > + if ((tmp & 0xc0) != 0x00) > + break; /* GPIO4x is not enable */ > + > + err = 0; /* GPIO4x set is all enabled */ > + break; > + case 5: > + superio_mask_outb(base, F81866_PORT_SEL_REG, BIT(3) | BIT(2), > + 0); > + tmp = superio_inb(base, F81866_MULTI_FUN1_REG); > + if ((tmp & 0x43) != 0x40) > + break; /* GPIO5x is not enable */ > + > + err = 0; /* GPIO5x set is all enabled */ > + break; > + case 6: > + superio_mask_outb(base, F81866_PORT_SEL_REG, BIT(3) | BIT(2) | > + BIT(0), 0); > + tmp = superio_inb(base, F81866_MULTI_FUN1_REG); > + if ((tmp & 0x4c) != 0x40) > + break; /* GPIO60~64 is not enable */ > + > + tmp = superio_inb(base, F81866_MULTI_FUN4_REG); > + if ((tmp & 0xe0) != 0xe0) > + break; /* GPIO65~67 is not enable */ > + > + err = 0; /* GPIO6x set is all enabled */ > + break; > + case 7: > + superio_mask_outb(base, F81866_PORT_SEL_REG, BIT(3) | BIT(2) | > + BIT(0), 0); > + tmp = superio_inb(base, F81866_MULTI_FUN1_REG); > + if ((tmp & 0x20) != 0x20) > + break; /* GPIO7x is not enable */ > + > + tmp = superio_inb(base, F81866_MULTI_FUN4_REG); > + if ((tmp & 0x01) != 0x00) > + break; /* GPIO70 is not enable */ > + > + err = 0; /* GPIO7x set is all enabled */ > + break; > + case 8: > + superio_mask_outb(base, F81866_PORT_SEL_REG, BIT(3) | BIT(2), > + 0); > + tmp = superio_inb(base, F81866_MULTI_FUN1_REG); > + if ((tmp & 0x20) != 0x20) > + break; /* GPIO8x is not enable */ > + > + err = 0; /* GPIO8x set is all enabled */ > + break; > + default: > + break; > + } > + > + superio_exit(base); > + return err; > +} > + > /* > * Platform device and driver. > */ > @@ -351,6 +485,15 @@ static int f7188x_gpio_probe(struct platform_device *pdev) > for (i = 0; i < data->nr_bank; i++) { > struct f7188x_gpio_bank *bank = &data->bank[i]; > > + /* > + * Dont export GPIO sysfs if pin set is not enable by MB Don't > + * manufacturer. > + */ > + if (sio->type == f81866 && f81866_verify_gpioset(sio->addr, i)) > + continue; > + > + dev_dbg(&pdev->dev, "%s: register GPIO%xx set\n", __func__, %x ? > + bank->chip.base >> 4); > bank->chip.dev = &pdev->dev; > bank->data = data; > > @@ -368,6 +511,11 @@ static int f7188x_gpio_probe(struct platform_device *pdev) > err_gpiochip: > for (i = i - 1; i >= 0; i--) { > struct f7188x_gpio_bank *bank = &data->bank[i]; > + > + /* Some GPIO is not export, not need to remove */ > + if (!bank->data || !bank->chip.dev) > + continue; > + > gpiochip_remove(&bank->chip); > } > > @@ -381,6 +529,11 @@ static int f7188x_gpio_remove(struct platform_device *pdev) > > for (i = 0; i < data->nr_bank; i++) { > struct f7188x_gpio_bank *bank = &data->bank[i]; > + > + /* Some GPIO is not export, not need to remove */ > + if (!bank->data || !bank->chip.dev) > + continue; > + > gpiochip_remove(&bank->chip); -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] gpio-f7188x: filter non-export gpio for F81866 2016-01-12 9:33 ` Andy Shevchenko @ 2016-01-13 2:03 ` Peter Hung 0 siblings, 0 replies; 7+ messages in thread From: Peter Hung @ 2016-01-13 2:03 UTC (permalink / raw) To: Andy Shevchenko Cc: Linus Walleij, Alexandre Courbot, linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org, tom_tsai, Peter H, Peter Hung Hi Andy, Andy Shevchenko 於 2016/1/12 下午 05:33 寫道: > On Tue, Jan 12, 2016 at 9:41 AM, Peter Hung <hpeter@gmail.com> wrote: >> Dont export gpios which not enabled by motherboard manufacturer. > > 'Do not' or 'Don't'. > > Perhaps 'GPIOs'. > > 'which are not enabled' OK. >> +#define F81866_PORT_SEL_REG 0x27 >> +#define F81866_MULTI_FUN1_REG 0x28 >> +#define F81866_MULTI_FUN3_REG 0x29 >> +#define F81866_MULTI_FUN4_REG 0x2B >> +#define F81866_GPIO_EN_REG 0x2C >> + > > Move this before IDs block. OK. >> +static inline void superio_mask_outb(int base, int reg, int mask, int val) > > Usually we named such as _update or _update_bits. > superio_update() OK >> +static int f81866_verify_gpioset(int base, int set) > > …_gpio_verify() looks more suitable to the existing scheme. OK >> + * manufacturer. >> + */ >> + if (sio->type == f81866 && f81866_verify_gpioset(sio->addr, i)) >> + continue; >> + >> + dev_dbg(&pdev->dev, "%s: register GPIO%xx set\n", __func__, > > %x ? Sorry for misunderstand. This section will print debug message as GPIO3x set if GPIO3 set enabled, so I used %xx. We are discussing about the patch with maintainer. It maybe drop if don't need filter. Thanks for your advices. -- With Best Regards, Peter Hung -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] gpio-f7188x: filter non-export gpio for F81866 2016-01-12 7:41 ` [PATCH 2/2] gpio-f7188x: filter non-export gpio for F81866 Peter Hung 2016-01-12 9:33 ` Andy Shevchenko @ 2016-01-12 15:36 ` Simon Guinot 2016-01-13 1:56 ` Peter Hung 1 sibling, 1 reply; 7+ messages in thread From: Simon Guinot @ 2016-01-12 15:36 UTC (permalink / raw) To: Peter Hung Cc: linus.walleij, gnurou, linux-gpio, linux-kernel, tom_tsai, peter_hong, Peter Hung [-- Attachment #1: Type: text/plain, Size: 6127 bytes --] On Tue, Jan 12, 2016 at 03:41:39PM +0800, Peter Hung wrote: > Dont export gpios which not enabled by motherboard manufacturer. > > Signed-off-by: Peter Hung <hpeter+linux_kernel@gmail.com> Hi Peter, > --- > drivers/gpio/gpio-f7188x.c | 153 +++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 153 insertions(+) > > diff --git a/drivers/gpio/gpio-f7188x.c b/drivers/gpio/gpio-f7188x.c > index a6a9641..f7fe7aa 100644 > --- a/drivers/gpio/gpio-f7188x.c > +++ b/drivers/gpio/gpio-f7188x.c > @@ -38,6 +38,12 @@ > #define SIO_F71889_ID 0x0909 /* F71889 chipset ID */ > #define SIO_F81866_ID 0x1010 /* F81866 chipset ID */ > > +#define F81866_PORT_SEL_REG 0x27 > +#define F81866_MULTI_FUN1_REG 0x28 > +#define F81866_MULTI_FUN3_REG 0x29 > +#define F81866_MULTI_FUN4_REG 0x2B > +#define F81866_GPIO_EN_REG 0x2C > + > enum chips { f71869, f71869a, f71882fg, f71889f, f81866 }; > > static const char * const f7188x_names[] = { > @@ -93,6 +99,15 @@ static inline void superio_outb(int base, int reg, int val) > outb(val, base + 1); > } > > +static inline void superio_mask_outb(int base, int reg, int mask, int val) > +{ > + u8 tmp; > + > + tmp = superio_inb(base, reg); > + tmp = (tmp & ~mask) | (val & mask); > + superio_outb(base, reg, tmp); > +} > + > static inline int superio_enter(int base) > { > /* Don't step on other drivers' I/O space by accident. */ > @@ -304,6 +319,125 @@ static void f7188x_gpio_set(struct gpio_chip *chip, unsigned offset, int value) > superio_exit(sio->addr); > } > > +static int f81866_verify_gpioset(int base, int set) > +{ > + int err; > + u8 tmp; > + > + err = superio_enter(base); > + if (err) > + return err; > + > + err = -ENODEV; > + > + switch (set) { > + case 0: > + superio_mask_outb(base, F81866_PORT_SEL_REG, BIT(3) | BIT(2) | > + BIT(0), 0); > + tmp = superio_inb(base, F81866_GPIO_EN_REG); > + if ((tmp & 0x1f) != 0x1f) > + break; /* one in GPIO00~GPIO04 is not enable */ > + > + tmp = superio_inb(base, F81866_MULTI_FUN1_REG); > + if ((tmp & 0xc) != 0x0) > + break; > + > + err = 0; /* GPIO0x set is all enabled */ > + break; > + case 1: > + superio_mask_outb(base, F81866_PORT_SEL_REG, BIT(3) | BIT(2) | > + BIT(0), BIT(2)); > + tmp = superio_inb(base, F81866_GPIO_EN_REG); > + if ((tmp & 0xef) != 0xef) > + break; /* one in GPIO10~GPIO17 is not enable */ > + > + tmp = superio_inb(base, F81866_MULTI_FUN3_REG); > + if ((tmp & 0x03) != 0x00) > + break; /* one in GPIO12~GPIO13 is not enable */ > + > + err = 0; /* GPIO1x set is all enabled */ > + break; > + case 2: > + superio_mask_outb(base, F81866_PORT_SEL_REG, BIT(3) | BIT(2) | > + BIT(0), BIT(3)); > + tmp = superio_inb(base, F81866_GPIO_EN_REG); > + if ((tmp & 0xff) != 0xff) > + break; /* one in GPIO20~GPIO27 is not enable */ > + > + tmp = superio_inb(base, F81866_MULTI_FUN3_REG); > + if ((tmp & 0x08) != 0x00) > + break; /* GPIO20 is not enable */ > + > + err = 0; /* GPIO2x set is all enabled */ > + break; > + case 3: > + superio_mask_outb(base, F81866_PORT_SEL_REG, BIT(0), 0); > + tmp = superio_inb(base, F81866_MULTI_FUN3_REG); > + if ((tmp & 0x30) != 0x00) > + break; /* GPIO3x is not enable */ > + > + err = 0; /* GPIO3x set is all enabled */ > + break; > + case 4: > + superio_mask_outb(base, F81866_PORT_SEL_REG, BIT(0), 0); > + tmp = superio_inb(base, F81866_MULTI_FUN3_REG); > + if ((tmp & 0xc0) != 0x00) > + break; /* GPIO4x is not enable */ > + > + err = 0; /* GPIO4x set is all enabled */ > + break; > + case 5: > + superio_mask_outb(base, F81866_PORT_SEL_REG, BIT(3) | BIT(2), > + 0); > + tmp = superio_inb(base, F81866_MULTI_FUN1_REG); > + if ((tmp & 0x43) != 0x40) > + break; /* GPIO5x is not enable */ > + > + err = 0; /* GPIO5x set is all enabled */ > + break; > + case 6: > + superio_mask_outb(base, F81866_PORT_SEL_REG, BIT(3) | BIT(2) | > + BIT(0), 0); > + tmp = superio_inb(base, F81866_MULTI_FUN1_REG); > + if ((tmp & 0x4c) != 0x40) > + break; /* GPIO60~64 is not enable */ > + > + tmp = superio_inb(base, F81866_MULTI_FUN4_REG); > + if ((tmp & 0xe0) != 0xe0) > + break; /* GPIO65~67 is not enable */ > + > + err = 0; /* GPIO6x set is all enabled */ > + break; > + case 7: > + superio_mask_outb(base, F81866_PORT_SEL_REG, BIT(3) | BIT(2) | > + BIT(0), 0); > + tmp = superio_inb(base, F81866_MULTI_FUN1_REG); > + if ((tmp & 0x20) != 0x20) > + break; /* GPIO7x is not enable */ > + > + tmp = superio_inb(base, F81866_MULTI_FUN4_REG); > + if ((tmp & 0x01) != 0x00) > + break; /* GPIO70 is not enable */ > + > + err = 0; /* GPIO7x set is all enabled */ > + break; > + case 8: > + superio_mask_outb(base, F81866_PORT_SEL_REG, BIT(3) | BIT(2), > + 0); > + tmp = superio_inb(base, F81866_MULTI_FUN1_REG); > + if ((tmp & 0x20) != 0x20) > + break; /* GPIO8x is not enable */ > + > + err = 0; /* GPIO8x set is all enabled */ > + break; > + default: > + break; > + } > + > + superio_exit(base); > + return err; > +} > + > /* > * Platform device and driver. > */ > @@ -351,6 +485,15 @@ static int f7188x_gpio_probe(struct platform_device *pdev) > for (i = 0; i < data->nr_bank; i++) { > struct f7188x_gpio_bank *bank = &data->bank[i]; > > + /* > + * Dont export GPIO sysfs if pin set is not enable by MB > + * manufacturer. What does MB stands for ? > + */ > + if (sio->type == f81866 && f81866_verify_gpioset(sio->addr, i)) > + continue; This whole filtering mechanism relies on the fact that the multiplexing configuration has been correctly applied by the BIOS (if applied at all). But I wonder if it is often the case. For example, I have several boards for which it is not true. And to make the GPIOs available, I need first to fix the multiplexing pin configuration of the Super I/O. Maybe it would be more correct to rely on the hardware description of a board (Device Tree or ACPI) to decide which GPIO bank can be enabled or not. Simon [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] gpio-f7188x: filter non-export gpio for F81866 2016-01-12 15:36 ` Simon Guinot @ 2016-01-13 1:56 ` Peter Hung 0 siblings, 0 replies; 7+ messages in thread From: Peter Hung @ 2016-01-13 1:56 UTC (permalink / raw) To: Simon Guinot Cc: linus.walleij, gnurou, linux-gpio, linux-kernel, tom_tsai, peter_hong, Peter Hung Hi Simon, >> @@ -351,6 +485,15 @@ static int f7188x_gpio_probe(struct platform_device *pdev) >> for (i = 0; i < data->nr_bank; i++) { >> struct f7188x_gpio_bank *bank = &data->bank[i]; >> >> + /* >> + * Dont export GPIO sysfs if pin set is not enable by MB >> + * manufacturer. > > What does MB stands for ? The MB is stands for "Motherboard", Due to the limit of 80 words length, I use the abbreviation. >> + */ >> + if (sio->type == f81866 && f81866_verify_gpioset(sio->addr, i)) >> + continue; > > This whole filtering mechanism relies on the fact that the multiplexing > configuration has been correctly applied by the BIOS (if applied at > all). But I wonder if it is often the case. For example, I have several > boards for which it is not true. And to make the GPIOs available, I need > first to fix the multiplexing pin configuration of the Super I/O. > > Maybe it would be more correct to rely on the hardware description of a > board (Device Tree or ACPI) to decide which GPIO bank can be enabled or > not. That's good for control by Device Tree or ACPI. IMO, we shouldn't export GPIOs not enabled if the BIOS had written wrong configuration to SuperIO, but it's only my opinion. Should I do filter for this? Thanks for your advices. -- With Best Regards, Peter Hung ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-01-13 2:03 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-01-12 7:41 [PATCH 0/2] gpio-f7188x: Add F81886 gpio function Peter Hung 2016-01-12 7:41 ` [PATCH 1/2] gpio-f7188x: add Fintek F81866 SuperIO support Peter Hung 2016-01-12 7:41 ` [PATCH 2/2] gpio-f7188x: filter non-export gpio for F81866 Peter Hung 2016-01-12 9:33 ` Andy Shevchenko 2016-01-13 2:03 ` Peter Hung 2016-01-12 15:36 ` Simon Guinot 2016-01-13 1:56 ` Peter Hung
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).