Re: [PATCH 1/3] gpio: zynq: use module_platform_driver to simplify the code
On Wed, Apr 14, 2021 at 4:45 PM Srinivas Neeli wrote:
>
> HI baratosz and Andy,
>
It's Bartosz. You literally just need to copy & paste the name from my email...
> > -Original Message-
> > From: Bartosz Golaszewski
> > Sent: Tuesday, April 13, 2021 4:14 PM
> > To: Andy Shevchenko
> > Cc: Srinivas Neeli ; linus.wall...@linaro.org; Michal
> > Simek
> > ; Shubhrajyoti Datta ; Srinivas
> > Goud ; linux-g...@vger.kernel.org; linux-arm-
> > ker...@lists.infradead.org; linux-kernel@vger.kernel.org; git
> >
> > Subject: Re: [PATCH 1/3] gpio: zynq: use module_platform_driver to simplify
> > the code
> >
> > On Sat, Apr 10, 2021 at 12:08 AM Andy Shevchenko
> > wrote:
> > >
> > >
> > >
> > > On Friday, April 9, 2021, Srinivas Neeli
> > > wrote:
> > >>
> > >> module_platform_driver() makes the code simpler by eliminating
> > >> boilerplate code.
> > >>
> > >> Signed-off-by: Srinivas Neeli
> > >> ---
> > >> drivers/gpio/gpio-zynq.c | 17 +
> > >> 1 file changed, 1 insertion(+), 16 deletions(-)
> > >>
> > >> diff --git a/drivers/gpio/gpio-zynq.c b/drivers/gpio/gpio-zynq.c
> > >> index 3521c1dc3ac0..bb1ac0c5cf26 100644
> > >> --- a/drivers/gpio/gpio-zynq.c
> > >> +++ b/drivers/gpio/gpio-zynq.c
> > >> @@ -1020,22 +1020,7 @@ static struct platform_driver zynq_gpio_driver
> > = {
> > >> .remove = zynq_gpio_remove,
> > >> };
> > >>
> > >> -/**
> > >> - * zynq_gpio_init - Initial driver registration call
> > >> - *
> > >> - * Return: value from platform_driver_register
> > >> - */
> > >> -static int __init zynq_gpio_init(void) -{
> > >> - return platform_driver_register(_gpio_driver);
> > >> -}
> > >> -postcore_initcall(zynq_gpio_init);
> > >
> > >
> > >
> > > It’s not an equivalent. Have you tested on actual hardware? If no, there
> > > is
> > no go for this change.
> > >
> >
> > Yep, this has been like this since the initial introduction of this driver.
> > Unfortunately there's no documented reason so unless we can test it, it has
> > to stay this way.
> >
> I tested driver, functionality wise everything working fine.
> Based on below conversation, I moved driver to module driver.
> https://lore.kernel.org/patchwork/patch/818202/
>
Andy: How about we give it a try then? If anyone yells, we'll just revert it.
> Thanks
> Srinivas Neeli
>
> > Bartosz
Bartosz
Re: [PATCH] eeprom: at24: avoid adjusting offset for 24AA025E{48, 64}
On Tue, Apr 13, 2021 at 1:03 PM wrote:
>
> On 12.04.2021 21:29, Bartosz Golaszewski wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> > content is safe
> >
> > On Mon, Apr 12, 2021 at 9:42 AM wrote:
> >>
> >> On 07.04.2021 21:37, Bartosz Golaszewski wrote:
> >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
> >>> the content is safe
> >>>
> >>> On Fri, Apr 2, 2021 at 3:24 PM Claudiu Beznea
> >>> wrote:
> >>>>
> >>>> Some EEPROMs could be used only for MAC storage. In this case the
> >>>> EEPROM areas where MACs resides could be modeled as NVMEM cells
> >>>> (directly via DT bindings) such that the already available networking
> >>>> infrastructure to read properly the MAC addresses (via
> >>>> of_get_mac_address()). Add "atmel,24mac02e4", "atmel,24mac02e4"
> >>>> compatible for the usage w/ 24AA025E{48, 64} type of EEPROMs and adapt
> >>>> the driver to not do offset adjustments.
> >>>>
> >>>> Signed-off-by: Claudiu Beznea
> >>>> ---
> >>>>
> >>>> Hi Bartosz,
> >>>>
> >>>> For the previously available compatibles the offset adjustment is done
> >>>> (probably for compatibility w/ old DT bindings?). In my scenario
> >>>> 24AA025E48
> >>>> is used in setup with macb driver which is calling of_get_mac_address()
> >>>> to get the proper NVMEM cell in EEPROM where the MAC resides and read
> >>>> directly from there. We modeled the EEPROM and NVMEM cell in DT as
> >>>> follows:
> >>>>
> >>>> {
> >>>> // ...
> >>>> eeprom0: eeprom0@52 {
> >>>> compatible = "atmel,24mac02e4";
> >
> > Can you point me to the datasheet for this model, google only directs
> > me to this very email.
>
> This is the datasheet:
> https://ww1.microchip.com/downloads/en/DeviceDoc/24AA02E48-24AA025E48-24AA02E64-24AA025E64-Data-Sheet-20002124H.pdf
>
> >
> >>From the device tree it looks as if it was just a regular 24c02 EEPROM
> > with MAC hard-coded at 250-255 bytes, is that right?
>
> Yes, the MAC is hard-coded at 250. But using "24c02" compatible will
> involve the offset adjustment in the driver (let me know if I missed
> something).
>
Something seems to be wrong. There's no offset adjustment for "24c02".
Have you tried running i2cdump on the EEPROM's address? Do you see the
MAC?
Bartosz
> Thank you,
> Claudiu
>
> >
> > Bartosz
> >
> >>>> #address-cells = <1>;
> >>>> #size-cells = <0>;
> >>>> reg = <0x52>;
> >>>> pagesize = <16>;
> >>>> size = <256>;
> >>>> status = "okay";
> >>>>
> >>>> eeprom0_eui48: eui48@fa {
> >>>> reg = <0xfa 0x6>;
> >>>> };
> >>>> };
> >>>> };
> >>>>
> >>>> {
> >>>> // ...
> >>>>
> >>>> nvmem-cells = <_eui48>;
> >>>> nvmem-cell-names = "mac-address";
> >>>>
> >>>> // ...
> >>>> };
> >>>>
> >>>>
> >>>> Let me know if some other approach needs to be taken into account in
> >>>> at24 driver for this to work.
> >>>>
> >>>> Thank you,
> >>>> Claudiu Beznea
> >>>>
> >>>
> >>> Hi Claudiu,
> >>>
> >>> First of all: any new compatibles need to go into the DT bindings
> >>> document.
> >>
> >> Agree! I missed this.
> >>
> >>>
> >>>
> >>>> drivers/misc/eeprom/at24.c | 69 ++
> >>>> 1 file changed, 40 insertions(+), 29 deletions(-)
> >>>>
> >>>> diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
> >>>> index 926408b41270..ae2fbcb5e83d 100644
> >>>> --- a/drivers/misc/eeprom/at24.c
> >>>> +++ b/drivers/misc/eeprom/at24.c
> >>>&
Re: [PATCH v19 4/6] misc: eeprom: at24: check suspend status before disable regulator
On Wed, Apr 14, 2021 at 7:29 PM Hsin-Yi Wang wrote:
>
> cd5676db0574 ("misc: eeprom: at24: support pm_runtime control") disables
> regulator in runtime suspend. If runtime suspend is called before
> regulator disable, it will results in regulator unbalanced disabling.
>
> Signed-off-by: Hsin-Yi Wang
Please add the Fixes tag.
> ---
> drivers/misc/eeprom/at24.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
> index 926408b41270..7a6f01ace78a 100644
> --- a/drivers/misc/eeprom/at24.c
> +++ b/drivers/misc/eeprom/at24.c
> @@ -763,7 +763,8 @@ static int at24_probe(struct i2c_client *client)
> at24->nvmem = devm_nvmem_register(dev, _config);
> if (IS_ERR(at24->nvmem)) {
> pm_runtime_disable(dev);
> - regulator_disable(at24->vcc_reg);
> + if (!pm_runtime_status_suspended(dev))
> + regulator_disable(at24->vcc_reg);
> return PTR_ERR(at24->nvmem);
> }
>
> @@ -774,7 +775,8 @@ static int at24_probe(struct i2c_client *client)
> err = at24_read(at24, 0, _byte, 1);
> if (err) {
> pm_runtime_disable(dev);
> - regulator_disable(at24->vcc_reg);
> + if (!pm_runtime_status_suspended(dev))
> + regulator_disable(at24->vcc_reg);
> return -ENODEV;
> }
>
> --
> 2.31.1.295.g9ea45b61b8-goog
>
Acked-by: Bartosz Golaszewski
[GIT PULL] gpio: fixes for v5.12-rc8
Linus, I waited until late with this non-urgent one hoping we'd get more fixes for this release cycle to go with it but nothing's coming up so please pull this single fix for an older problem with the sysfs interface. Bartosz The following changes since commit 6cb59afe9e5b45a035bd6b97da6593743feefc72: gpiolib: Assign fwnode to parent's if no primary one provided (2021-03-16 10:18:08 +0100) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git tags/gpio-fixes-for-v5.12-rc8 for you to fetch changes up to 23cf00ddd2e1aacf1873e43f5e0c519c120daf7a: gpio: sysfs: Obey valid_mask (2021-03-31 20:32:38 +0200) gpio fixes for v5.12-rc8 - do not allow exporting GPIO lines which were marked invalid by the driver Matti Vaittinen (1): gpio: sysfs: Obey valid_mask drivers/gpio/gpiolib-sysfs.c | 8 1 file changed, 8 insertions(+)
Re: [PATCH 1/3] gpio: zynq: use module_platform_driver to simplify the code
On Sat, Apr 10, 2021 at 12:08 AM Andy Shevchenko
wrote:
>
>
>
> On Friday, April 9, 2021, Srinivas Neeli wrote:
>>
>> module_platform_driver() makes the code simpler by eliminating
>> boilerplate code.
>>
>> Signed-off-by: Srinivas Neeli
>> ---
>> drivers/gpio/gpio-zynq.c | 17 +
>> 1 file changed, 1 insertion(+), 16 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-zynq.c b/drivers/gpio/gpio-zynq.c
>> index 3521c1dc3ac0..bb1ac0c5cf26 100644
>> --- a/drivers/gpio/gpio-zynq.c
>> +++ b/drivers/gpio/gpio-zynq.c
>> @@ -1020,22 +1020,7 @@ static struct platform_driver zynq_gpio_driver = {
>> .remove = zynq_gpio_remove,
>> };
>>
>> -/**
>> - * zynq_gpio_init - Initial driver registration call
>> - *
>> - * Return: value from platform_driver_register
>> - */
>> -static int __init zynq_gpio_init(void)
>> -{
>> - return platform_driver_register(_gpio_driver);
>> -}
>> -postcore_initcall(zynq_gpio_init);
>
>
>
> It’s not an equivalent. Have you tested on actual hardware? If no, there is
> no go for this change.
>
Yep, this has been like this since the initial introduction of this
driver. Unfortunately there's no documented reason so unless we can
test it, it has to stay this way.
Bartosz
Re: [PATCH] gpio: mxs: remove useless function
On Mon, Apr 12, 2021 at 4:16 AM Jiapeng Chong
wrote:
>
> Fix the following gcc warning:
>
> drivers/gpio/gpio-mxs.c:63:19: warning: kernel/sys_ni.cunused function
> 'is_imx28_gpio'.
>
> Reported-by: Abaci Robot
> Signed-off-by: Jiapeng Chong
> ---
> drivers/gpio/gpio-mxs.c | 5 -
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mxs.c b/drivers/gpio/gpio-mxs.c
> index dfc0c1e..524b668 100644
> --- a/drivers/gpio/gpio-mxs.c
> +++ b/drivers/gpio/gpio-mxs.c
> @@ -60,11 +60,6 @@ static inline int is_imx23_gpio(struct mxs_gpio_port *port)
> return port->devid == IMX23_GPIO;
> }
>
> -static inline int is_imx28_gpio(struct mxs_gpio_port *port)
> -{
> - return port->devid == IMX28_GPIO;
> -}
> -
> /* Note: This driver assumes 32 GPIOs are handled in one register */
>
> static int mxs_gpio_set_irq_type(struct irq_data *d, unsigned int type)
> --
> 1.8.3.1
>
Patch applied, thanks!
Bartosz
Re: [PATCH v2 4/6] devres: handle zero size in devm_kmalloc()
On Sun, Apr 11, 2021 at 5:21 AM Dmitry Torokhov wrote: > > Hi Bartosz, > > On Mon, Jun 29, 2020 at 1:56 PM Bartosz Golaszewski wrote: > > > > From: Bartosz Golaszewski > > > > Make devm_kmalloc() behave similarly to non-managed kmalloc(): return > > ZERO_SIZE_PTR when requested size is 0. Update devm_kfree() to handle > > this case. > > This is wrong if you consider devm_krealloc API that you added. The > premise of devm_krealloc() is that it does not disturb devres "stack", > however in this case there is no entry in the stack. Consider: > > ptr = devm_kzalloc(dev, 0, GFP_KERNEL); > ... > more devm API calls > ... > > /* This allocation will be on top of devm stack, not bottom ! */ > ptr = devm_krealloc(dev, ptr, 16, GFP_KERNEL); > > And also: > > ptr = devm_kzalloc(dev, 16, GFP_KERNEL); > ... > more devm API calls > ... > /* Here we lose out position */ > ptr = devm_krealloc(dev, ptr, 0, GFP_KERNEL); > ... > /* and now our memory allocation will be released first */ > ptr = devm_krealloc(dev, ptr, 16, GFP_KERNEL); > > > IMO special-casing 0-size allocations for managed memory allocations > should not be done. > > Thanks. > > -- > Dmitry You're right about the ordering being lost. At the same time allocating 0 bytes is quite a special case and should result in returning ZERO_SIZE_PTR as the fault dump resulting from its dereference will indicate what the bug is. I need to give it a thought because I'm not yet sure what the right solution would be. Let me get back to you. Bartosz
Re: [PATCH] eeprom: at24: avoid adjusting offset for 24AA025E{48, 64}
On Mon, Apr 12, 2021 at 9:42 AM wrote:
>
> On 07.04.2021 21:37, Bartosz Golaszewski wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> > content is safe
> >
> > On Fri, Apr 2, 2021 at 3:24 PM Claudiu Beznea
> > wrote:
> >>
> >> Some EEPROMs could be used only for MAC storage. In this case the
> >> EEPROM areas where MACs resides could be modeled as NVMEM cells
> >> (directly via DT bindings) such that the already available networking
> >> infrastructure to read properly the MAC addresses (via
> >> of_get_mac_address()). Add "atmel,24mac02e4", "atmel,24mac02e4"
> >> compatible for the usage w/ 24AA025E{48, 64} type of EEPROMs and adapt
> >> the driver to not do offset adjustments.
> >>
> >> Signed-off-by: Claudiu Beznea
> >> ---
> >>
> >> Hi Bartosz,
> >>
> >> For the previously available compatibles the offset adjustment is done
> >> (probably for compatibility w/ old DT bindings?). In my scenario 24AA025E48
> >> is used in setup with macb driver which is calling of_get_mac_address()
> >> to get the proper NVMEM cell in EEPROM where the MAC resides and read
> >> directly from there. We modeled the EEPROM and NVMEM cell in DT as
> >> follows:
> >>
> >> {
> >> // ...
> >> eeprom0: eeprom0@52 {
> >> compatible = "atmel,24mac02e4";
Can you point me to the datasheet for this model, google only directs
me to this very email.
>From the device tree it looks as if it was just a regular 24c02 EEPROM
with MAC hard-coded at 250-255 bytes, is that right?
Bartosz
> >> #address-cells = <1>;
> >> #size-cells = <0>;
> >> reg = <0x52>;
> >> pagesize = <16>;
> >> size = <256>;
> >> status = "okay";
> >>
> >> eeprom0_eui48: eui48@fa {
> >> reg = <0xfa 0x6>;
> >> };
> >> };
> >> };
> >>
> >> {
> >> // ...
> >>
> >> nvmem-cells = <_eui48>;
> >> nvmem-cell-names = "mac-address";
> >>
> >> // ...
> >> };
> >>
> >>
> >> Let me know if some other approach needs to be taken into account in
> >> at24 driver for this to work.
> >>
> >> Thank you,
> >> Claudiu Beznea
> >>
> >
> > Hi Claudiu,
> >
> > First of all: any new compatibles need to go into the DT bindings document.
>
> Agree! I missed this.
>
> >
> >
> >> drivers/misc/eeprom/at24.c | 69 ++
> >> 1 file changed, 40 insertions(+), 29 deletions(-)
> >>
> >> diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
> >> index 926408b41270..ae2fbcb5e83d 100644
> >> --- a/drivers/misc/eeprom/at24.c
> >> +++ b/drivers/misc/eeprom/at24.c
> >> @@ -123,17 +123,19 @@ MODULE_PARM_DESC(at24_write_timeout, "Time (in ms)
> >> to try writes (default 25)");
> >> struct at24_chip_data {
> >> u32 byte_len;
> >> u8 flags;
> >> + u8 adjoff;
> >> void (*read_post)(unsigned int off, char *buf, size_t count);
> >> };
> >>
> >> -#define AT24_CHIP_DATA(_name, _len, _flags)\
> >> +#define AT24_CHIP_DATA(_name, _len, _flags, _adjoff) \
> >> static const struct at24_chip_data _name = {\
> >> - .byte_len = _len, .flags = _flags, \
> >> + .byte_len = _len, .flags = _flags, .adjoff = _adjoff, \
> >> }
> >>
> >> -#define AT24_CHIP_DATA_CB(_name, _len, _flags, _read_post) \
> >> +#define AT24_CHIP_DATA_CB(_name, _len, _flags, _adjoff, _read_post)\
> >> static const struct at24_chip_data _name = {\
> >> .byte_len = _len, .flags = _flags, \
> >> + .adjoff = _adjoff, \
> >> .read_post = _read_post,\
> >> }
> >>
> >> @@ -158,48 +160,52 @@ static void at24_read_post_vaio(unsigned int off,
> >
Re: [PATCH stable] gpiolib: Read "gpio-line-names" from a firmware node
On Sat, Apr 10, 2021 at 2:07 PM Greg Kroah-Hartman
wrote:
>
> On Sat, Apr 10, 2021 at 11:09:19AM +0200, Bartosz Golaszewski wrote:
> > From: Andy Shevchenko
> >
> > On STM32MP1, the GPIO banks are subnodes of pin-controller@50002000,
> > see arch/arm/boot/dts/stm32mp151.dtsi. The driver for
> > pin-controller@50002000 is in drivers/pinctrl/stm32/pinctrl-stm32.c
> > and iterates over all of its DT subnodes when registering each GPIO
> > bank gpiochip. Each gpiochip has:
> >
> > - gpio_chip.parent = dev,
> > where dev is the device node of the pin controller
> > - gpio_chip.of_node = np,
> > which is the OF node of the GPIO bank
> >
> > Therefore, dev_fwnode(chip->parent) != of_fwnode_handle(chip.of_node),
> > i.e. pin-controller@50002000 != pin-controller@50002000/gpio@5000*000.
> >
> > The original code behaved correctly, as it extracted the "gpio-line-names"
> > from of_fwnode_handle(chip.of_node) = pin-controller@50002000/gpio@5000*000.
> >
> > To achieve the same behaviour, read property from the firmware node.
> >
> > Fixes: 7cba1a4d5e162 ("gpiolib: generalize devprop_gpiochip_set_names() for
> > device properties")
> > Cc: sta...@vger.kernel.org
> > Reported-by: Marek Vasut
> > Reported-by: Roman Guskov
> > Signed-off-by: Andy Shevchenko
> > Tested-by: Marek Vasut
> > Reviewed-by: Marek Vasut
> > Signed-off-by: Bartosz Golaszewski
> > ---
> > Hi Greg,
> >
> > This patch somehow got lost and never made its way into stable. Could you
> > please apply it?
>
> This has been added and removed more times than I can remember already.
>
> Are you all _SURE_ this is safe for a stable kernel release? Look in
> the archives for complaints when we added this in the past.
>
> thanks,
>
> greg k-h
IIRC it fixed the stm32mp1 problem but exposed a different problem
breaking other users until Andy fixed the deeper issue elsewhere.
It's now fine to apply it.
Bartosz
[PATCH stable] gpiolib: Read "gpio-line-names" from a firmware node
From: Andy Shevchenko
On STM32MP1, the GPIO banks are subnodes of pin-controller@50002000,
see arch/arm/boot/dts/stm32mp151.dtsi. The driver for
pin-controller@50002000 is in drivers/pinctrl/stm32/pinctrl-stm32.c
and iterates over all of its DT subnodes when registering each GPIO
bank gpiochip. Each gpiochip has:
- gpio_chip.parent = dev,
where dev is the device node of the pin controller
- gpio_chip.of_node = np,
which is the OF node of the GPIO bank
Therefore, dev_fwnode(chip->parent) != of_fwnode_handle(chip.of_node),
i.e. pin-controller@50002000 != pin-controller@50002000/gpio@5000*000.
The original code behaved correctly, as it extracted the "gpio-line-names"
from of_fwnode_handle(chip.of_node) = pin-controller@50002000/gpio@5000*000.
To achieve the same behaviour, read property from the firmware node.
Fixes: 7cba1a4d5e162 ("gpiolib: generalize devprop_gpiochip_set_names() for
device properties")
Cc: sta...@vger.kernel.org
Reported-by: Marek Vasut
Reported-by: Roman Guskov
Signed-off-by: Andy Shevchenko
Tested-by: Marek Vasut
Reviewed-by: Marek Vasut
Signed-off-by: Bartosz Golaszewski
---
Hi Greg,
This patch somehow got lost and never made its way into stable. Could you
please apply it?
Thanks,
Bartosz
drivers/gpio/gpiolib.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 4253837f870b..7ec0822c0505 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -367,22 +367,18 @@ static int gpiochip_set_desc_names(struct gpio_chip *gc)
*
* Looks for device property "gpio-line-names" and if it exists assigns
* GPIO line names for the chip. The memory allocated for the assigned
- * names belong to the underlying software node and should not be released
+ * names belong to the underlying firmware node and should not be released
* by the caller.
*/
static int devprop_gpiochip_set_names(struct gpio_chip *chip)
{
struct gpio_device *gdev = chip->gpiodev;
- struct device *dev = chip->parent;
+ struct fwnode_handle *fwnode = dev_fwnode(>dev);
const char **names;
int ret, i;
int count;
- /* GPIO chip may not have a parent device whose properties we inspect.
*/
- if (!dev)
- return 0;
-
- count = device_property_string_array_count(dev, "gpio-line-names");
+ count = fwnode_property_string_array_count(fwnode, "gpio-line-names");
if (count < 0)
return 0;
@@ -396,7 +392,7 @@ static int devprop_gpiochip_set_names(struct gpio_chip
*chip)
if (!names)
return -ENOMEM;
- ret = device_property_read_string_array(dev, "gpio-line-names",
+ ret = fwnode_property_read_string_array(fwnode, "gpio-line-names",
names, count);
if (ret < 0) {
dev_warn(>dev, "failed to read GPIO line names\n");
--
2.30.1
Re: [PATCH v1 1/1] gpiolib: Read "gpio-line-names" from a firmware node
On Sat, Apr 10, 2021 at 2:46 AM Marek Vasut wrote:
>
> On 3/15/21 6:04 PM, Andy Shevchenko wrote:
> > On Mon, Mar 15, 2021 at 6:49 PM Bartosz Golaszewski
> > wrote:
> >>
> >> On Mon, Mar 15, 2021 at 3:34 PM Andy Shevchenko
> >> wrote:
> >>>
> >>> On Mon, Mar 15, 2021 at 03:04:37PM +0100, Bartosz Golaszewski wrote:
> >>>> On Mon, Mar 15, 2021 at 1:50 PM Andy Shevchenko
> >>>> wrote:
> >>>>>
> >>>>> On Mon, Mar 15, 2021 at 12:16:26PM +0200, Andy Shevchenko wrote:
> >>>>>> On Mon, Mar 15, 2021 at 10:01:47AM +0100, Bartosz Golaszewski wrote:
> >>>>>>> On Fri, Mar 5, 2021 at 1:03 PM Andy Shevchenko
> >>>>>>> wrote:
> >>>>>>
> >>>>>>> Unfortunately while this may fix the particular use-case on STM32, it
> >>>>>>> breaks all other users as the 'gpio-line-names' property doesn't live
> >>>>>>> on dev_fwnode(>dev) but on dev_fwnode(chip->parent).
> >>>>>>>
> >>>>>>> How about we first look for this property on the latter and only if
> >>>>>>> it's not present descend down to the former fwnode?
> >>>>>>
> >>>>>> Oops, I have tested on x86 and it worked the same way.
> >>>>>>
> >>>>>> Lemme check this, but I think the issue rather in ordering when we
> >>>>>> apply fwnode
> >>>>>> to the newly created device and when we actually retrieve
> >>>>>> gpio-line-names
> >>>>>> property.
> >>>>>
> >>>>> Hmm... I can't see how it's possible can be. Can you provide a platform
> >>>>> name
> >>>>> and pointers to the DTS that has been broken by the change?
> >>>>>
> >>>>
> >>>> I noticed it with gpio-mockup (libgpiod tests failed on v5.12-rc3) and
> >>>> the WiP gpio-sim - but it's the same on most DT platforms. The node
> >>>> that contains the `gpio-line-names` is the one associated with the
> >>>> platform device passed to the GPIO driver. The gpiolib then creates
> >>>> another struct device that becomes the child of that node but it
> >>>> doesn't copy the parent's properties to it (nor should it).
> >>>>
> >>>> Every driver that reads device properties does it from the parent
> >>>> device, not the one in gdev - whether it uses of_, fwnode_ or generic
> >>>> device_ properties.
> >>>
> >>> What you are telling contradicts with the idea of copying parent's fwnode
> >>> (or OF node) in the current code.
> >>>
> >>
> >> Ha! While the OF node of the parent device is indeed assigned to the
> >> gdev's dev, the same isn't done in the core code for fwnodes and
> >> simulated chips don't have an associated OF node, so this is the
> >> culprit I suppose.
> >
> > Close, but not fully correct.
> > First of all it depends on the OF / ACPI / platform enumeration.
> > Second, we are talking about secondary fwnode in the case where it happens.
> >
> > I'm in the middle of debugging this, I'll come up with something soon I
> > believe.
>
> Was there ever any follow up on this ?
>
> I would like to point out that on STM32MP1 in Linux 5.10.y, the
> gpio-line-names are still broken, and a revert of "gpiolib: generalize
> devprop_gpiochip_set_names() for device properties" is still necessary.
Yes, Andy has fixed that in commit b41ba2ec54a7 ("gpiolib: Read
"gpio-line-names" from a firmware node") but for some reason this has
never made its way into stable. I'll resend it.
Bartosz
Re: [PATCH] gpio: gpio-it87: remove unused code
On Tue, Apr 6, 2021 at 10:35 AM Simon Guinot wrote:
>
> On Tue, Apr 06, 2021 at 03:20:39PM +0800, Jiapeng Chong wrote:
> > Fix the following clang warning:
> >
> > drivers/gpio/gpio-it87.c:128:20: warning: unused function 'superio_outw'
> > [-Wunused-function].
> >
> > Reported-by: Abaci Robot
> > Signed-off-by: Jiapeng Chong
>
> Acked-by: Simon Guinot
>
> > ---
> > drivers/gpio/gpio-it87.c | 8
> > 1 file changed, 8 deletions(-)
> >
> > diff --git a/drivers/gpio/gpio-it87.c b/drivers/gpio/gpio-it87.c
> > index 8f1be34..f332341 100644
> > --- a/drivers/gpio/gpio-it87.c
> > +++ b/drivers/gpio/gpio-it87.c
> > @@ -125,14 +125,6 @@ static inline int superio_inw(int reg)
> > return val;
> > }
> >
> > -static inline void superio_outw(int val, int reg)
> > -{
> > - outb(reg++, REG);
> > - outb(val >> 8, VAL);
> > - outb(reg, REG);
> > - outb(val, VAL);
> > -}
> > -
> > static inline void superio_set_mask(int mask, int reg)
> > {
> > u8 curr_val = superio_inb(reg);
> > --
> > 1.8.3.1
Patch applied, thanks!
Bartosz
Re: [PATCH v2] gpio: gpio-104-dio-48e: Fixed coding style issues (revised)
On Thu, Apr 8, 2021 at 5:54 PM Barney Goette wrote: > > Fixed multiple bare uses of 'unsigned' without 'int'. > Fixed space around "*" operator. > Fixed function parameter alignment to opening parenthesis. > Reported by checkpatch. > > Signed-off-by: Barney Goette > Acked-by: William Breathitt Gray > --- Changed the commit message to `gpio: 104-dio-48e: Fix coding style issues` and applied, thanks! Bartosz
Re: [PATCH v4] gpio: mpc8xxx: Add ACPI support
On Thu, Apr 8, 2021 at 9:20 AM Linus Walleij wrote: > > On Tue, Apr 6, 2021 at 3:49 AM Ran Wang wrote: > > > Could this version be accepted, or any comment/suggestion? > > Andy says yes, then it is a yes :) > FWIW > Acked-by: Linus Walleij > > Yours, > Linus Walleij Applied, thanks! Bartosz
Re: [PATCH v1 1/1] gpio: sim: Initialize attribute allocated on the heap
On Thu, Apr 8, 2021 at 6:24 PM Andy Shevchenko
wrote:
>
> The attributes on the heap must be initialized before use.
> Neglecting that will produce an Oops in some configurations:
>
> BUG: key 000800eba398 has not been registered!
>
> Initialize attribute allocated on the heap.
>
> Fixes: 3f0279eb9e37 ("gpio: sim: new testing module")
> Reported-by: Naresh Kamboju
> Tested-by: Naresh Kamboju
> Signed-off-by: Andy Shevchenko
> ---
> drivers/gpio/gpio-sim.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpio/gpio-sim.c b/drivers/gpio/gpio-sim.c
> index ea17289a869c..92493b98c51b 100644
> --- a/drivers/gpio/gpio-sim.c
> +++ b/drivers/gpio/gpio-sim.c
> @@ -284,6 +284,7 @@ static int gpio_sim_setup_sysfs(struct gpio_sim_chip
> *chip)
> line_attr->offset = i;
>
> dev_attr = _attr->dev_attr;
> + sysfs_attr_init(_attr->attr);
>
> dev_attr->attr.name = devm_kasprintf(dev, GFP_KERNEL,
> "gpio%u", i);
> --
> 2.30.2
>
So writing tests really serves a purpose, heh? :)
Thanks for the patch and QA Andy and Naresh, patch applied.
Bartosz
Re: [PATCH] eeprom: at24: avoid adjusting offset for 24AA025E{48, 64}
On Fri, Apr 2, 2021 at 3:24 PM Claudiu Beznea
wrote:
>
> Some EEPROMs could be used only for MAC storage. In this case the
> EEPROM areas where MACs resides could be modeled as NVMEM cells
> (directly via DT bindings) such that the already available networking
> infrastructure to read properly the MAC addresses (via
> of_get_mac_address()). Add "atmel,24mac02e4", "atmel,24mac02e4"
> compatible for the usage w/ 24AA025E{48, 64} type of EEPROMs and adapt
> the driver to not do offset adjustments.
>
> Signed-off-by: Claudiu Beznea
> ---
>
> Hi Bartosz,
>
> For the previously available compatibles the offset adjustment is done
> (probably for compatibility w/ old DT bindings?). In my scenario 24AA025E48
> is used in setup with macb driver which is calling of_get_mac_address()
> to get the proper NVMEM cell in EEPROM where the MAC resides and read
> directly from there. We modeled the EEPROM and NVMEM cell in DT as
> follows:
>
> {
> // ...
> eeprom0: eeprom0@52 {
> compatible = "atmel,24mac02e4";
> #address-cells = <1>;
> #size-cells = <0>;
> reg = <0x52>;
> pagesize = <16>;
> size = <256>;
> status = "okay";
>
> eeprom0_eui48: eui48@fa {
> reg = <0xfa 0x6>;
> };
> };
> };
>
> {
> // ...
>
> nvmem-cells = <_eui48>;
> nvmem-cell-names = "mac-address";
>
> // ...
> };
>
>
> Let me know if some other approach needs to be taken into account in
> at24 driver for this to work.
>
> Thank you,
> Claudiu Beznea
>
Hi Claudiu,
First of all: any new compatibles need to go into the DT bindings document.
> drivers/misc/eeprom/at24.c | 69 ++
> 1 file changed, 40 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
> index 926408b41270..ae2fbcb5e83d 100644
> --- a/drivers/misc/eeprom/at24.c
> +++ b/drivers/misc/eeprom/at24.c
> @@ -123,17 +123,19 @@ MODULE_PARM_DESC(at24_write_timeout, "Time (in ms) to
> try writes (default 25)");
> struct at24_chip_data {
> u32 byte_len;
> u8 flags;
> + u8 adjoff;
> void (*read_post)(unsigned int off, char *buf, size_t count);
> };
>
> -#define AT24_CHIP_DATA(_name, _len, _flags)\
> +#define AT24_CHIP_DATA(_name, _len, _flags, _adjoff) \
> static const struct at24_chip_data _name = {\
> - .byte_len = _len, .flags = _flags, \
> + .byte_len = _len, .flags = _flags, .adjoff = _adjoff, \
> }
>
> -#define AT24_CHIP_DATA_CB(_name, _len, _flags, _read_post) \
> +#define AT24_CHIP_DATA_CB(_name, _len, _flags, _adjoff, _read_post)\
> static const struct at24_chip_data _name = {\
> .byte_len = _len, .flags = _flags, \
> + .adjoff = _adjoff, \
> .read_post = _read_post,\
> }
>
> @@ -158,48 +160,52 @@ static void at24_read_post_vaio(unsigned int off, char
> *buf, size_t count)
> }
>
> /* needs 8 addresses as A0-A2 are ignored */
> -AT24_CHIP_DATA(at24_data_24c00, 128 / 8, AT24_FLAG_TAKE8ADDR);
> +AT24_CHIP_DATA(at24_data_24c00, 128 / 8, AT24_FLAG_TAKE8ADDR, 0);
> /* old variants can't be handled with this generic entry! */
> -AT24_CHIP_DATA(at24_data_24c01, 1024 / 8, 0);
> +AT24_CHIP_DATA(at24_data_24c01, 1024 / 8, 0, 0);
> AT24_CHIP_DATA(at24_data_24cs01, 16,
> - AT24_FLAG_SERIAL | AT24_FLAG_READONLY);
> -AT24_CHIP_DATA(at24_data_24c02, 2048 / 8, 0);
> + AT24_FLAG_SERIAL | AT24_FLAG_READONLY, 0);
> +AT24_CHIP_DATA(at24_data_24c02, 2048 / 8, 0, 0);
> AT24_CHIP_DATA(at24_data_24cs02, 16,
> - AT24_FLAG_SERIAL | AT24_FLAG_READONLY);
> + AT24_FLAG_SERIAL | AT24_FLAG_READONLY, 0);
> AT24_CHIP_DATA(at24_data_24mac402, 48 / 8,
> - AT24_FLAG_MAC | AT24_FLAG_READONLY);
> + AT24_FLAG_MAC | AT24_FLAG_READONLY, 1);
> AT24_CHIP_DATA(at24_data_24mac602, 64 / 8,
> - AT24_FLAG_MAC | AT24_FLAG_READONLY);
> + AT24_FLAG_MAC | AT24_FLAG_READONLY, 1);
> +AT24_CHIP_DATA(at24_data_24mac02e4, 48 / 8,
> + AT24_FLAG_MAC | AT24_FLAG_READONLY, 0);
> +AT24_CHIP_DATA(at24_data_24mac02e6, 64 / 8,
> + AT24_FLAG_MAC | AT24_FLAG_READONLY, 0);
> /* spd is a 24c02 in memory DIMMs */
> AT24_CHIP_DATA(at24_data_spd, 2048 / 8,
> - AT24_FLAG_READONLY | AT24_FLAG_IRUGO);
> + AT24_FLAG_READONLY | AT24_FLAG_IRUGO, 0);
> /* 24c02_vaio is a 24c02 on some Sony laptops */
> AT24_CHIP_DATA_CB(at24_data_24c02_vaio, 2048 / 8,
> - AT24_FLAG_READONLY | AT24_FLAG_IRUGO,
> + AT24_FLAG_READONLY | AT24_FLAG_IRUGO, 0,
> at24_read_post_vaio);
> -AT24_CHIP_DATA(at24_data_24c04, 4096 / 8, 0);
>
Re: [PATCH 02/12] ARM: davinci: Constify the software nodes
On Tue, Apr 6, 2021 at 9:38 PM Wolfram Sang wrote: > > On Mon, Mar 29, 2021 at 01:50:37PM +0300, Heikki Krogerus wrote: > > Additional device properties are always just a part of a > > software fwnode. If the device properties are constant, the > > software node can also be constant. > > > > Signed-off-by: Heikki Krogerus > > Cc: Sekhar Nori > > Cc: Bartosz Golaszewski > > I like to apply it soon. Can we get an ack, please? > Looks good to me. Acked-by: Bartosz Golaszewski
Re: [PATCH v3 3/3] gpio: xilinx: Utilize generic bitmap_get_value and _set_value
On Thu, Apr 1, 2021 at 1:16 PM Syed Nayyar Waris wrote:
>
> On Wed, Mar 31, 2021 at 8:56 PM Srinivas Neeli wrote:
> >
> > Hi,
> >
> > > -Original Message-
> > > From: Bartosz Golaszewski
> > > Sent: Friday, March 26, 2021 10:58 PM
> > > To: Michal Simek
> > > Cc: Syed Nayyar Waris ; Srinivas Neeli
> > > ; Andy Shevchenko
> > > ; William Breathitt Gray
> > > ; Arnd Bergmann ; Robert
> > > Richter ; Linus Walleij ;
> > > Masahiro Yamada ; Andrew Morton
> > > ; Zhang Rui ; Daniel
> > > Lezcano ; Amit Kucheria
> > > ; Linux-Arch ;
> > > linux-gpio ; LKML > > ker...@vger.kernel.org>; arm-soc ;
> > > linux-pm ; Srinivas Goud
> > > Subject: Re: [PATCH v3 3/3] gpio: xilinx: Utilize generic
> > > bitmap_get_value and
> > > _set_value
> > >
> > > On Mon, Mar 8, 2021 at 8:13 AM Michal Simek
> > > wrote:
> > > >
> > > >
> > > >
> > > > On 3/6/21 3:06 PM, Syed Nayyar Waris wrote:
> > > > > This patch reimplements the xgpio_set_multiple() function in
> > > > > drivers/gpio/gpio-xilinx.c to use the new generic functions:
> > > > > bitmap_get_value() and bitmap_set_value(). The code is now simpler
> > > > > to read and understand. Moreover, instead of looping for each bit in
> > > > > xgpio_set_multiple() function, now we can check each channel at a
> > > > > time and save cycles.
> > > > >
> > > > > Cc: Bartosz Golaszewski
> > > > > Cc: Michal Simek
> > > > > Signed-off-by: Syed Nayyar Waris
> > > > > Acked-by: William Breathitt Gray
> > > > > ---
> > > > > drivers/gpio/gpio-xilinx.c | 63
> > > > > +++---
> > > > > 1 file changed, 32 insertions(+), 31 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpio/gpio-xilinx.c b/drivers/gpio/gpio-xilinx.c
> > > > > index be539381fd82..8445e69cf37b 100644
> > > > > --- a/drivers/gpio/gpio-xilinx.c
> > > > > +++ b/drivers/gpio/gpio-xilinx.c
> > > > > @@ -15,6 +15,7 @@
> > > > > #include
> > > > > #include
> > > > > #include
> > > > > +#include "gpiolib.h"
> > > > >
> > > > > /* Register Offset Definitions */
> > > > > #define XGPIO_DATA_OFFSET (0x0)/* Data register */
> > > > > @@ -141,37 +142,37 @@ static void xgpio_set_multiple(struct
> > > > > gpio_chip *gc, unsigned long *mask, {
> > > > > unsigned long flags;
> > > > > struct xgpio_instance *chip = gpiochip_get_data(gc);
> > > > > - int index = xgpio_index(chip, 0);
> > > > > - int offset, i;
> > > > > -
> > > > > - spin_lock_irqsave(>gpio_lock[index], flags);
> > > > > -
> > > > > - /* Write to GPIO signals */
> > > > > - for (i = 0; i < gc->ngpio; i++) {
> > > > > - if (*mask == 0)
> > > > > - break;
> > > > > - /* Once finished with an index write it out to the
> > > > > register */
> > > > > - if (index != xgpio_index(chip, i)) {
> > > > > - xgpio_writereg(chip->regs + XGPIO_DATA_OFFSET +
> > > > > -index * XGPIO_CHANNEL_OFFSET,
> > > > > -chip->gpio_state[index]);
> > > > > - spin_unlock_irqrestore(>gpio_lock[index],
> > > > > flags);
> > > > > - index = xgpio_index(chip, i);
> > > > > - spin_lock_irqsave(>gpio_lock[index],
> > > > > flags);
> > > > > - }
> > > > > - if (__test_and_clear_bit(i, mask)) {
> > > > > - offset = xgpio_offset(chip, i);
> > > > > - if (test_bit(i, bits))
> > > > > - chip->gpio_state[index] |= BIT(offset);
> > > > > - else
> > > > > - chip->gpio_state[index] &= ~BIT(offset);
> > > > > - }
> > > > > - }
> > > >
Re: [PATCH RFC/RFT 1/1] misc: add simple logic analyzer using polling
On Thu, Apr 1, 2021 at 3:08 PM Linus Walleij wrote:
>
> On Tue, Mar 30, 2021 at 10:58 AM Wolfram Sang
> wrote:
>
> > This is a simple logic analyzer using GPIO polling. It comes with a
> > script to isolate a CPU for polling. While this is definately not a
> > production level analyzer, it can be a helpful first view when remote
> > debugging. Read the documentation for details.
> >
> > Signed-off-by: Wolfram Sang
>
> I am a great supporter of this idea.
>
> When we created gpiod_get_array_value() and friends, the idea
> was exactly to be able to do things like this. It's a good way to
> utilize the fact that several GPIO lines can often be read from a single
> register read.
>
> > +i2c-analyzer {
> > +compatible = "gpio-logic-analyzer";
> > +probe-gpios = < 21 GPIO_OPEN_DRAIN>, < 4
> > GPIO_OPEN_DRAIN>;
> > +probe-names = "SCL", "SDA";
> > +};
> > +
> > +The binding documentation is in the ``misc`` folder of the Kernel binding
> > +documentation.
> (...)
> > +++ b/Documentation/devicetree/bindings/misc/gpio-logic-analyzer.yaml
>
> When other debugging tools for GPIO got DT bindings it was concluded that
> it is possible to create bindings like this for debugging without even
> specifying
> any formal bindings. They are just for debugging after all.
>
> Personally I like the bindings anyway.
>
> > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
>
> I would consider housing this tool under drivers/gpio actually.
> We have other funky things like gpio-sim and gpio-aggregator
> so why not.
>
We have actually created a sub-menu for "Virtual GPIO drivers".
> I would create a Kconfig menu with "GPIO hardware hacking tools".
>
> But Bartosz would need to agree on that idea.
>
It's perfect! If we ever get something like a generic bitbanging
driver or something, it could go in there too.
Bart
> > +config GPIO_LOGIC_ANALYZER
> > + tristate "Simple GPIO logic analyzer"
> > + depends on GPIOLIB || COMPILE_TEST
> > + help
>
> depends on EXPERT
>
> I would say. Definitely not something for the average user.
>
> Yours,
> Linus Walleij
Re: [PATCH 1/3] docs: gpio: mockup: Fix parameter name
On Wed, Mar 31, 2021 at 2:53 PM Alexander Dahl wrote:
>
> Hello Andy,
>
> Am Wed, Mar 31, 2021 at 03:27:05PM +0300 schrieb Andy Shevchenko:
> > On Mon, Mar 29, 2021 at 2:18 PM Alexander Dahl wrote:
> > >
> > > Module probing with the parameter documented yielded this in kernel log:
> > >
> > > gpio_mockup: unknown parameter 'gpio_named_lines' ignored
> > >
> > > The parameter documented did not match the parameter actually
> > > implemented with commit 8a68ea00a62e ("gpio: mockup: implement naming
> > > the lines") long before introducing the documentation.
> > >
> > > Fixes: commit 2fd1abe99e5f ("Documentation: gpio: add documentation for
> > > gpio-mockup")
> >
> > Alexander, in the entire series you are using the wrong format for the
> > Fixes tag.
> > I highly recommend to add in your .gitconfig file an alias:
> > one = show -s --pretty='format:%h (\"%s\")'
>
> You're right. Sorry, I messed things up. I first had that line without
> the additional "commit", and added it because I thought checkpatch
> complained (which it probably did not).
>
> The actual format is documented of course:
>
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
>
> I actually have a slightly different setting for pretty.fixes in my
> ~/.gitconfig for other demands. I'll go and make that project
> dependent now.
>
> Thanks for pointing this out.
>
> Greets
> Alex
>
> >
> > `git one ` will give you proper value to refer to the
> > commit in question.
> >
> > --
> > With Best Regards,
> > Andy Shevchenko
FYI no need to resend, I fixed it in my tree.
Bart
Re: [PATCH 1/2] gpio: sysfs: Obey valid_mask
On Wed, Mar 31, 2021 at 2:25 PM Andy Shevchenko wrote: > > On Wed, Mar 31, 2021 at 10:58 AM Bartosz Golaszewski > wrote: > > > > On Mon, Mar 29, 2021 at 1:41 PM Matti Vaittinen > > wrote: > > > > > > Do not allow exporting GPIOs which are set invalid > > > by the driver's valid mask. > > > > > > Fixes: 726cb3ba49692bdae6caff457755e7cdb432efa4 > > I have just noticed that this is invalid format for the Fixes tag > (luckily, haha, due to a blank line it's not recognized as a tag!). > > Matti, I highly recommend to add in your .gitconfig file an alias: > one = show -s --pretty='format:%h (\"%s\")' > > Bart, there are real Fixes tag issues from another series. I will > comment there as well to let an author know. > > -- Eek, sorry I should have looked more carefully. I'll fix it in my tree. Bartosz
Re: [PATCH 2/2] gpiolib: Allow drivers to return EOPNOTSUPP from config
On Tue, Mar 30, 2021 at 6:32 AM Matti Vaittinen wrote: > > Morning Folks, > > On Mon, 2021-03-29 at 16:30 +0300, Andy Shevchenko wrote: > > On Mon, Mar 29, 2021 at 03:20:07PM +0300, Matti Vaittinen wrote: > > > On Mon, 2021-03-29 at 14:59 +0300, Andy Shevchenko wrote: > > > > On Mon, Mar 29, 2021 at 2:43 PM Matti Vaittinen > > > > wrote: > > > > > The checkpacth instructs to switch from ENOSUPP to EOPNOTSUPP. > > > > > > WARNING: ENOTSUPP is not a SUSV4 error code, prefer > > > > > > EOPNOTSUPP > > > > > > > > > > Make the gpiolib allow drivers to return both so driver > > > > > developers > > > > > can avoid one of the checkpatch complaints. > > > > > > > > Internally we are fine to use the ENOTSUPP. > > > > Checkpatch false positives there. > > > > > > I agree. OTOH, the checkpatch check makes sense to user-visible > > > stuff. > > > Yet, the checkpatch has hard time guessing what is user-visible - > > > so it > > > probably is easiest to nag about all ENOTSUPP uses as it does now. > > > > > > > I doubt we need this change. Rather checkpatch should rephrase > > > > this > > > > to > > > > point out that this is only applicable to _user-visible_ error > > > > path. > > > > Cc'ed Joe. > > > > > > Yes, thanks for pulling Joe in. > > > > > > Anyways, no matter what the warning says, all false positives are > > > annoying. I don't see why we should stay with ENOTSUPP in gpiolib? > > > (other than the burden of changing it). > > > > For sake of the changing we are not changing the code. > No. But for the sake of making it better / more consistent :) > > Anyway - after giving this second thought (thanks Andy for provoking me > to thinking this further) - I do agree with Andy that this particular > change is bad. More I think of this, less I like the idea of having two > separate return values to indicate the same thing. So we should support > only one which makes my patch terrible. > > For the sake of consistency it would be cleaner to use same, single > value, for same error both inside the gpiolib and at user-interface. > That would be EOPNOTSUPP. As I said, having two separate error codes to > indicate same thing is confusing. Now the confusion is at the boundary > of gpiolib and user-land. Please educate me - is there difference in > the meaning of ENOTSUPP and EOPNOTSUPP or are they really indicating > the same thing? If yes, then yes - correct fix would be to use only one > and ditch the other. Whether the amount of work is such it is > practically worth is another topic - but that would be the right thing > to do (tm). > In user-space there's no ENOTSUPP but there's ENOTSUP which is defined in most sane toolchains as: #define ENOTSUP EOPNOTSUPP While ENOTSUPP is not the same number as EOPNOTSUPP. So to answer the question: they mean the same thing in the kernel but not to user-space. We must never return ENOTSUPP to user-space because it doesn't know it. Bartosz > > > > > But I have no strong opinion on this. All options I see have > > > downsides. > > > > > > Accepting both ENOTSUPP and EOPNOTSUPP is the easy way to avoid > > > allowing checkpatch warnings - but I admit it isn't stylish. > > > > I think the error code which is Linux kernel internal is for a > > reason. > > If so, then the current checkpatch warning is even more questionable. > > > > > > Converting all ENOTSUPP cases inside gpiolib to EOPNOTSUPP is > > > teodious > > > although end result might be nicer. > > > > Why? You still missed the justification except satisfying some tool > > that gives > > you false positives. We don't do that. It's the tool that has to be > > fixed / > > amended. > > > > > Leaving it as is gives annoying false-positives to driver > > > developers. > > > > > > My personal preference was this patch - others can have other view > > > like > > > Andy does. I'll leave this to community/maintainers to evaluate :) > > > > This patch misses documentation fixes, for example. > > Valid point. > > > Also, do you suggest that we have to do the same in entire pin > > control > > subsystem? > > After reading/writing this, I am unsure. This is why the discussion is > good :) I don't see why we should have two separate error codes for > same thing - but as you put it: > > > I think the error code which is Linux kernel internal is for a > > reason. > > not all of us thinks the same. So maybe I just don't get it? :) > > Best Regards > Matti Vaittinen > >
Re: [PATCH 1/2] gpio: sysfs: Obey valid_mask
On Mon, Mar 29, 2021 at 1:41 PM Matti Vaittinen wrote: > > Do not allow exporting GPIOs which are set invalid > by the driver's valid mask. > > Fixes: 726cb3ba49692bdae6caff457755e7cdb432efa4 > > Signed-off-by: Matti Vaittinen > --- Applied, thanks! Bart
Re: [PATCH v6 0/2] Add Realtek Otto GPIO support
On Tue, Mar 30, 2021 at 7:48 PM Sander Vanheule wrote: > > Add support for the GPIO controller employed by Realtek in multiple series of > MIPS SoCs. These include the supported RTL838x and RTL839x. The register > layout > also matches the one found in the GPIO controller of other (Lexra-based) SoCs > such as RTL8196E, RTL8197D, and RTL8197F. > > For the platform name 'otto', I am not aware of any official resources as to > what hardware this specifically applies to. However, in all of the GPL > archives > we've received, from vendors using compatible SoCs in their design, the > platform under the MIPS architecture is referred to by this name. > > The GPIO ports have been tested on a Zyxel GS1900-8 (RTL8380), and Zyxel > GS1900-48 (RTL8393). Furthermore, the GPIO ports and interrupt controller have > been tested on a Netgear GS110TPPv1 (RTL8381). > > Changes in v6: > - Use devm_gpiochip_add_data() > - Code style for reading ngpios, header order > - Add Andy's Reviewed-by tag > > Changes in v5: > - Edited code comments > - Fold functions that were used only once or twice (ISR/IMR accessors) > - Drop trivial functions for line to port/pin calculations > - Use gpio_irq_chip->init_hw() to initialise IRQ registers > - Invert GPIO_INTERRUPTS flag to GPIO_INTERRUPTS_DISABLED > - Support building as module > - Add Rob's Reviewed-by tag > > Changes in v4: > - Fix pointer notation style > - Drop unused read_u16_reg() function > - Drop 'inline' specifier from functions > > Changes in v3: > - Remove OF dependencies in driver probe > - Don't accept IRQ_TYPE_NONE as a valid interrupt type > - Remove (now unused) dev property from control structure > - Use u8/u16 port registers, instead of raw u32 registers > - Use 'line' name for gpiochip, 'port' and 'pin' names for hardware > - Renamed DT bindings file > - Dropped fallback-only DT compatible > - Various code style clean-ups > > Changes in v2: > - Clarify structure and usage of IMR registers > - Added Linus' Reviewed-by tags > > Sander Vanheule (2): > dt-bindings: gpio: Binding for Realtek Otto GPIO > gpio: Add Realtek Otto GPIO support > > .../bindings/gpio/realtek,otto-gpio.yaml | 78 + > drivers/gpio/Kconfig | 13 + > drivers/gpio/Makefile | 1 + > drivers/gpio/gpio-realtek-otto.c | 325 ++ > 4 files changed, 417 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/gpio/realtek,otto-gpio.yaml > create mode 100644 drivers/gpio/gpio-realtek-otto.c > > -- > 2.30.2 > Series applied, thanks! Bartosz
Re: [PATCH 3/3] docs: kernel-parameters: Add gpio_mockup_named_lines
On Mon, Mar 29, 2021 at 1:17 PM Alexander Dahl wrote:
>
> Missing since introduced in the driver.
>
> Fixes: commit 8a68ea00a62e ("gpio: mockup: implement naming the lines")
> Signed-off-by: Alexander Dahl
> ---
Applied, thanks!
Bartosz
Re: [PATCH 2/3] docs: kernel-parameters: Move gpio-mockup for alphabetic order
On Mon, Mar 29, 2021 at 1:17 PM Alexander Dahl wrote:
>
> Fixes: commit 0f98dd1b27d2 ("gpio/mockup: add virtual gpio device")
> Signed-off-by: Alexander Dahl
> ---
Applied, thanks!
Bartosz
Re: [PATCH 1/3] docs: gpio: mockup: Fix parameter name
On Mon, Mar 29, 2021 at 1:17 PM Alexander Dahl wrote:
>
> Module probing with the parameter documented yielded this in kernel log:
>
> gpio_mockup: unknown parameter 'gpio_named_lines' ignored
>
> The parameter documented did not match the parameter actually
> implemented with commit 8a68ea00a62e ("gpio: mockup: implement naming
> the lines") long before introducing the documentation.
>
> Fixes: commit 2fd1abe99e5f ("Documentation: gpio: add documentation for
> gpio-mockup")
> Signed-off-by: Alexander Dahl
> ---
> Documentation/admin-guide/gpio/gpio-mockup.rst | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/admin-guide/gpio/gpio-mockup.rst
> b/Documentation/admin-guide/gpio/gpio-mockup.rst
> index 9fa1618b3adc..e3cafb4451aa 100644
> --- a/Documentation/admin-guide/gpio/gpio-mockup.rst
> +++ b/Documentation/admin-guide/gpio/gpio-mockup.rst
> @@ -27,7 +27,7 @@ module.
> the second 16 and the third 4. The base GPIO for the third chip is
> set
> to 405 while for two first chips it will be assigned automatically.
>
> -gpio_named_lines
> +gpio_mockup_named_lines
>
> This parameter doesn't take any arguments. It lets the driver know
> that
> GPIO lines exposed by it should be named.
> --
> 2.20.1
>
This was already fixed by Andy.
Bartosz
[tip: irq/core] genirq/irq_sim: Shrink devm_irq_domain_create_sim()
The following commit has been merged into the irq/core branch of tip:
Commit-ID: 883ccef355b910398b99dfaf96d40557479a7e9b
Gitweb:
https://git.kernel.org/tip/883ccef355b910398b99dfaf96d40557479a7e9b
Author:Bartosz Golaszewski
AuthorDate:Mon, 01 Mar 2021 15:26:59 +01:00
Committer: Thomas Gleixner
CommitterDate: Tue, 30 Mar 2021 13:21:27 +02:00
genirq/irq_sim: Shrink devm_irq_domain_create_sim()
The custom devres structure manages only a single pointer which can
can be achieved by using devm_add_action_or_reset() as well which
makes the code simpler.
[ tglx: Fixed return value handling - found by smatch ]
Signed-off-by: Bartosz Golaszewski
Signed-off-by: Thomas Gleixner
Link: https://lore.kernel.org/r/20210301142659.8971-1-b...@bgdev.pl
---
kernel/irq/irq_sim.c | 31 ---
1 file changed, 12 insertions(+), 19 deletions(-)
diff --git a/kernel/irq/irq_sim.c b/kernel/irq/irq_sim.c
index 4800660..6e935d4 100644
--- a/kernel/irq/irq_sim.c
+++ b/kernel/irq/irq_sim.c
@@ -24,10 +24,6 @@ struct irq_sim_irq_ctx {
struct irq_sim_work_ctx *work_ctx;
};
-struct irq_sim_devres {
- struct irq_domain *domain;
-};
-
static void irq_sim_irqmask(struct irq_data *data)
{
struct irq_sim_irq_ctx *irq_ctx = irq_data_get_irq_chip_data(data);
@@ -216,11 +212,11 @@ void irq_domain_remove_sim(struct irq_domain *domain)
}
EXPORT_SYMBOL_GPL(irq_domain_remove_sim);
-static void devm_irq_domain_release_sim(struct device *dev, void *res)
+static void devm_irq_domain_remove_sim(void *data)
{
- struct irq_sim_devres *this = res;
+ struct irq_domain *domain = data;
- irq_domain_remove_sim(this->domain);
+ irq_domain_remove_sim(domain);
}
/**
@@ -238,20 +234,17 @@ struct irq_domain *devm_irq_domain_create_sim(struct
device *dev,
struct fwnode_handle *fwnode,
unsigned int num_irqs)
{
- struct irq_sim_devres *dr;
+ struct irq_domain *domain;
+ int ret;
- dr = devres_alloc(devm_irq_domain_release_sim,
- sizeof(*dr), GFP_KERNEL);
- if (!dr)
- return ERR_PTR(-ENOMEM);
+ domain = irq_domain_create_sim(fwnode, num_irqs);
+ if (IS_ERR(domain))
+ return domain;
- dr->domain = irq_domain_create_sim(fwnode, num_irqs);
- if (IS_ERR(dr->domain)) {
- devres_free(dr);
- return dr->domain;
- }
+ ret = devm_add_action_or_reset(dev, devm_irq_domain_remove_sim, domain);
+ if (ret)
+ return ERR_PTR(ret);
- devres_add(dev, dr);
- return dr->domain;
+ return domain;
}
EXPORT_SYMBOL_GPL(devm_irq_domain_create_sim);
[tip: irq/core] genirq/irq_sim: Shrink devm_irq_domain_create_sim()
The following commit has been merged into the irq/core branch of tip:
Commit-ID: e6d46eded43dacf6370a7ae70f927ef4692cfcab
Gitweb:
https://git.kernel.org/tip/e6d46eded43dacf6370a7ae70f927ef4692cfcab
Author:Bartosz Golaszewski
AuthorDate:Mon, 01 Mar 2021 15:26:59 +01:00
Committer: Thomas Gleixner
CommitterDate: Mon, 29 Mar 2021 15:36:00 +02:00
genirq/irq_sim: Shrink devm_irq_domain_create_sim()
The custom devres structure manages only a single pointer which can
can be achieved by using devm_add_action_or_reset() as well which
makes the code simpler.
Signed-off-by: Bartosz Golaszewski
Signed-off-by: Thomas Gleixner
Link: https://lore.kernel.org/r/20210301142659.8971-1-b...@bgdev.pl
---
kernel/irq/irq_sim.c | 31 ---
1 file changed, 12 insertions(+), 19 deletions(-)
diff --git a/kernel/irq/irq_sim.c b/kernel/irq/irq_sim.c
index 4800660..ee40a84 100644
--- a/kernel/irq/irq_sim.c
+++ b/kernel/irq/irq_sim.c
@@ -24,10 +24,6 @@ struct irq_sim_irq_ctx {
struct irq_sim_work_ctx *work_ctx;
};
-struct irq_sim_devres {
- struct irq_domain *domain;
-};
-
static void irq_sim_irqmask(struct irq_data *data)
{
struct irq_sim_irq_ctx *irq_ctx = irq_data_get_irq_chip_data(data);
@@ -216,11 +212,11 @@ void irq_domain_remove_sim(struct irq_domain *domain)
}
EXPORT_SYMBOL_GPL(irq_domain_remove_sim);
-static void devm_irq_domain_release_sim(struct device *dev, void *res)
+static void devm_irq_domain_remove_sim(void *data)
{
- struct irq_sim_devres *this = res;
+ struct irq_domain *domain = data;
- irq_domain_remove_sim(this->domain);
+ irq_domain_remove_sim(domain);
}
/**
@@ -238,20 +234,17 @@ struct irq_domain *devm_irq_domain_create_sim(struct
device *dev,
struct fwnode_handle *fwnode,
unsigned int num_irqs)
{
- struct irq_sim_devres *dr;
+ struct irq_domain *domain;
+ int ret;
- dr = devres_alloc(devm_irq_domain_release_sim,
- sizeof(*dr), GFP_KERNEL);
- if (!dr)
- return ERR_PTR(-ENOMEM);
+ domain = irq_domain_create_sim(fwnode, num_irqs);
+ if (IS_ERR(domain))
+ return domain;
- dr->domain = irq_domain_create_sim(fwnode, num_irqs);
- if (IS_ERR(dr->domain)) {
- devres_free(dr);
- return dr->domain;
- }
+ ret = devm_add_action_or_reset(dev, devm_irq_domain_remove_sim, domain);
+ if (!ret)
+ return ERR_PTR(ret);
- devres_add(dev, dr);
- return dr->domain;
+ return domain;
}
EXPORT_SYMBOL_GPL(devm_irq_domain_create_sim);
Re: [PATCH v3 3/3] gpio: xilinx: Utilize generic bitmap_get_value and _set_value
On Mon, Mar 8, 2021 at 8:13 AM Michal Simek wrote:
>
>
>
> On 3/6/21 3:06 PM, Syed Nayyar Waris wrote:
> > This patch reimplements the xgpio_set_multiple() function in
> > drivers/gpio/gpio-xilinx.c to use the new generic functions:
> > bitmap_get_value() and bitmap_set_value(). The code is now simpler
> > to read and understand. Moreover, instead of looping for each bit
> > in xgpio_set_multiple() function, now we can check each channel at
> > a time and save cycles.
> >
> > Cc: Bartosz Golaszewski
> > Cc: Michal Simek
> > Signed-off-by: Syed Nayyar Waris
> > Acked-by: William Breathitt Gray
> > ---
> > drivers/gpio/gpio-xilinx.c | 63 +++---
> > 1 file changed, 32 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/gpio/gpio-xilinx.c b/drivers/gpio/gpio-xilinx.c
> > index be539381fd82..8445e69cf37b 100644
> > --- a/drivers/gpio/gpio-xilinx.c
> > +++ b/drivers/gpio/gpio-xilinx.c
> > @@ -15,6 +15,7 @@
> > #include
> > #include
> > #include
> > +#include "gpiolib.h"
> >
> > /* Register Offset Definitions */
> > #define XGPIO_DATA_OFFSET (0x0)/* Data register */
> > @@ -141,37 +142,37 @@ static void xgpio_set_multiple(struct gpio_chip *gc,
> > unsigned long *mask,
> > {
> > unsigned long flags;
> > struct xgpio_instance *chip = gpiochip_get_data(gc);
> > - int index = xgpio_index(chip, 0);
> > - int offset, i;
> > -
> > - spin_lock_irqsave(>gpio_lock[index], flags);
> > -
> > - /* Write to GPIO signals */
> > - for (i = 0; i < gc->ngpio; i++) {
> > - if (*mask == 0)
> > - break;
> > - /* Once finished with an index write it out to the register */
> > - if (index != xgpio_index(chip, i)) {
> > - xgpio_writereg(chip->regs + XGPIO_DATA_OFFSET +
> > -index * XGPIO_CHANNEL_OFFSET,
> > -chip->gpio_state[index]);
> > - spin_unlock_irqrestore(>gpio_lock[index],
> > flags);
> > - index = xgpio_index(chip, i);
> > - spin_lock_irqsave(>gpio_lock[index], flags);
> > - }
> > - if (__test_and_clear_bit(i, mask)) {
> > - offset = xgpio_offset(chip, i);
> > - if (test_bit(i, bits))
> > - chip->gpio_state[index] |= BIT(offset);
> > - else
> > - chip->gpio_state[index] &= ~BIT(offset);
> > - }
> > - }
> > -
> > - xgpio_writereg(chip->regs + XGPIO_DATA_OFFSET +
> > -index * XGPIO_CHANNEL_OFFSET, chip->gpio_state[index]);
> > -
> > - spin_unlock_irqrestore(>gpio_lock[index], flags);
> > + u32 *const state = chip->gpio_state;
> > + unsigned int *const width = chip->gpio_width;
> > +
> > + DECLARE_BITMAP(old, 64);
> > + DECLARE_BITMAP(new, 64);
> > + DECLARE_BITMAP(changed, 64);
> > +
> > + spin_lock_irqsave(>gpio_lock[0], flags);
> > + spin_lock(>gpio_lock[1]);
> > +
> > + bitmap_set_value(old, 64, state[0], width[0], 0);
> > + bitmap_set_value(old, 64, state[1], width[1], width[0]);
> > + bitmap_replace(new, old, bits, mask, gc->ngpio);
> > +
> > + bitmap_set_value(old, 64, state[0], 32, 0);
> > + bitmap_set_value(old, 64, state[1], 32, 32);
> > + state[0] = bitmap_get_value(new, 0, width[0]);
> > + state[1] = bitmap_get_value(new, width[0], width[1]);
> > + bitmap_set_value(new, 64, state[0], 32, 0);
> > + bitmap_set_value(new, 64, state[1], 32, 32);
> > + bitmap_xor(changed, old, new, 64);
> > +
> > + if (((u32 *)changed)[0])
> > + xgpio_writereg(chip->regs + XGPIO_DATA_OFFSET,
> > + state[0]);
> > + if (((u32 *)changed)[1])
> > + xgpio_writereg(chip->regs + XGPIO_DATA_OFFSET +
> > + XGPIO_CHANNEL_OFFSET, state[1]);
> > +
> > + spin_unlock(>gpio_lock[1]);
> > + spin_unlock_irqrestore(>gpio_lock[0], flags);
> > }
> >
> > /**
> >
>
> Srinivas N: Can you please test this code?
>
> Thanks,
> Michal
Hey, any chance of getting that Tested-by?
Bart
Re: [PATCH v2 10/17] gpio: support ROHM BD71815 GPOs
On Tue, Mar 23, 2021 at 10:57 AM Vaittinen, Matti wrote: > > > On Tue, 2021-01-19 at 12:07 +0100, Bartosz Golaszewski wrote: > > On Tue, Jan 19, 2021 at 8:23 AM Matti Vaittinen > > wrote: > > > Support GPO(s) found from ROHM BD71815 power management IC. The IC > > > has two > > > GPO pins but only one is properly documented in data-sheet. The > > > driver > > > exposes by default only the documented GPO. The second GPO is > > > connected to > > > E5 pin and is marked as GND in data-sheet. Control for this > > > undocumented > > > pin can be enabled using a special DT property. > > > > > > This driver is derived from work by Peter Yang < > > > yang...@embest-tech.com> > > > although not so much of original is left. > > > > > > Signed-off-by: Matti Vaittinen > > > > Hi Matti, > > > > looks great, just a couple nits. > > Hello Bartosz, > > I think fixed all the nits to v3. Can I translate this to an ack? (I > will respin the series as I guess the regulator part may have fallen > through the cracks so I'd like to add the relevant acks :] ) > > Best Regards > Matti Vaittinen Yes: Acked-by: Bartosz Golaszewski
Re: [PATCH v5 00/11] gpio: implement the configfs testing module
On Thu, Mar 25, 2021 at 10:29 AM Linus Walleij wrote: > > On Mon, Mar 22, 2021 at 3:32 PM Bartosz Golaszewski wrote: > > > FYI The configfs patches from this series have been on the mailing > > list for months (long before the GPIO part) and have been re-sent > > several times. You have neither acked or opposed these changes. I > > don't want to delay the new testing driver anymore so I intend to > > apply the entire series and take it upstream through the GPIO tree by > > the end of this week. > > I say go ahead. > Acked-by: Linus Walleij > > Yours, > Linus Walleij Series applied, thanks for the reviews! Bartosz
Re: [PATCH v9 02/22] gpio: regmap: set gpio_chip of_node
On Wed, Mar 24, 2021 at 9:19 AM Álvaro Fernández Rojas wrote: > > This is needed for properly registering GPIO regmap as a child of a regmap > pin controller. > > Signed-off-by: Álvaro Fernández Rojas > Reviewed-by: Michael Walle > Reviewed-by: Andy Shevchenko > --- Acked-by: Bartosz Golaszewski
Re: [PATCH v9 01/22] gpio: guard gpiochip_irqchip_add_domain() with GPIOLIB_IRQCHIP
On Wed, Mar 24, 2021 at 9:19 AM Álvaro Fernández Rojas
wrote:
>
> The current code doesn't check if GPIOLIB_IRQCHIP is enabled, which results in
> a compilation error when trying to build gpio-regmap if CONFIG_GPIOLIB_IRQCHIP
> isn't enabled.
>
> Fixes: 6a45b0e2589f ("gpiolib: Introduce gpiochip_irqchip_add_domain()")
> Suggested-by: Michael Walle
> Signed-off-by: Álvaro Fernández Rojas
> Reviewed-by: Linus Walleij
> Reviewed-by: Michael Walle
> ---
Acked-by: Bartosz Golaszewski
I suppose this will go through the pinctrl tree.
Bartosz
Re: [PATCH RESEND 0/7] gpio-rockchip driver
On Wed, Mar 24, 2021 at 7:50 AM jay...@rock-chips.com wrote: > > I'm forget to send-to include Bartosz, I'll remember in next version > > -- > jay...@rock-chips.com > >Separate gpio driver from pinctrl driver, and support v2 controller. > > > >Jianqun Xu (7): > > pinctrl/rockchip: separate struct rockchip_pin_bank to a head file > > pinctrl/pinctrl-rockchip.h: add pinctrl device to gpio bank struct > > gpio: separate gpio driver from pinctrl-rockchip driver > > gpio/rockchip: use struct rockchip_gpio_regs for gpio controller > > gpio/rockchip: support next version gpio controller > > gpio/rockchip: always enable clock for gpio controller > > gpio/rockchip: drop irq_gc_lock/irq_gc_unlock for irq set type > > > > drivers/gpio/Kconfig | 8 + > > drivers/gpio/Makefile | 1 + > > drivers/gpio/gpio-rockchip.c | 758 > > drivers/pinctrl/pinctrl-rockchip.c | 909 + > > drivers/pinctrl/pinctrl-rockchip.h | 286 + > > 5 files changed, 1072 insertions(+), 890 deletions(-) > > create mode 100644 drivers/gpio/gpio-rockchip.c > > create mode 100644 drivers/pinctrl/pinctrl-rockchip.h > > > >-- > >2.25.1 > > > > > > I don't even have this in my inbox so I can't review it. Bartosz
Re: [PATCH] linux/gpio/driver.h: some edits for clarity
On Tue, Mar 23, 2021 at 11:19 PM Randy Dunlap wrote: > > Fix a few typos and some punctuation. > Also, change CONFIG_OF to CONFIG_OF_GPIO in one comment. > > Signed-off-by: Randy Dunlap > Cc: Linus Walleij > Cc: Bartosz Golaszewski > Cc: linux-g...@vger.kernel.org > --- Applied with a slightly improved commit message. Bartosz
Re: [PATCH] tools: gpio-utils: fix various kernel-doc warnings
On Thu, Mar 25, 2021 at 6:56 PM Randy Dunlap wrote: > > Fix several problems in kernel-doc notation in gpio-utils.c. > > gpio-utils.c:37: warning: Incorrect use of kernel-doc format: * > gpiotools_request_line() - request gpio lines in a gpiochip > gpio-utils.c:61: warning: expecting prototype for doc(). Prototype was for > gpiotools_request_line() instead > gpio-utils.c:265: warning: Excess function parameter 'value' description in > 'gpiotools_sets' > gpio-utils.c:1: warning: 'gpiotools_request_lines' not found > > Signed-off-by: Randy Dunlap > Cc: Bartosz Golaszewski > Cc: Linus Walleij > Cc: linux-g...@vger.kernel.org > --- Applied, thanks! Bartosz
Re: [PATCH v5 00/11] gpio: implement the configfs testing module
On Mon, Mar 15, 2021 at 10:14 AM Bartosz Golaszewski wrote:
>
> From: Bartosz Golaszewski
>
> This series adds a new GPIO testing module based on configfs committable items
> and sysfs. The goal is to provide a testing driver that will be configurable
> at runtime (won't need module reload) and easily extensible. The control over
> the attributes is also much more fine-grained than in gpio-mockup.
>
> This series also contains a respin of the patches I sent separately to the
> configfs maintainers - these patches implement the concept of committable
> items that was well defined for a long time but never actually completed.
>
> Apart from the new driver itself, its selftests and the configfs patches, this
> series contains some changes to the bitmap API - most importantly: it adds
> devres managed variants of bitmap_alloc() and bitmap_zalloc().
>
> v1 -> v2:
> - add selftests for gpio-sim
> - add helper programs for selftests
> - update the configfs rename callback to work with the new API introduced in
> v5.11
> - fix a missing quote in the documentation
> - use !! whenever using bits operation that are required to return 0 or 1
> - use provided bitmap API instead of reimplementing copy or fill operations
> - fix a deadlock in gpio_sim_direction_output()
> - add new read-only configfs attributes for mapping of configfs items to GPIO
> device names
> - and address other minor issues pointed out in reviews of v1
>
> v2 -> v3:
> - use devm_bitmap_alloc() instead of the zalloc variant if we're initializing
> the bitmap with 1s
> - drop the patch exporting device_is_bound()
> - don't return -ENODEV from dev_nam and chip_name configfs attributes, return
> a string indicating that the device is not available yet ('n/a')
> - fix indentation where it makes sense
> - don't protect IDA functions which use their own locking and where it's not
> needed
> - use kmemdup() instead of kzalloc() + memcpy()
> - collected review tags
> - minor coding style fixes
>
> v3 -> v4:
> - return 'none' instead of 'n/a' from dev_name and chip_name before the device
> is registered
> - use sysfs_emit() instead of s*printf()
> - drop GPIO_SIM_MAX_PROP as it's only used in an array's definition where it's
> fine to hardcode the value
>
> v4 -> v5:
> - export devm bitmap functions with EXPORT_SYMBOL_GPL() instead of a simple
> EXPORT_SYMBOL()
>
> Bartosz Golaszewski (11):
> configfs: increase the item name length
> configfs: use (1UL << bit) for internal flags
> configfs: implement committable items
> samples: configfs: add a committable group
> lib: bitmap: remove the 'extern' keyword from function declarations
> lib: bitmap: order includes alphabetically
> lib: bitmap: provide devm_bitmap_alloc() and devm_bitmap_zalloc()
> gpio: sim: new testing module
> selftests: gpio: provide a helper for reading chip info
> selftests: gpio: add a helper for reading GPIO line names
> selftests: gpio: add test cases for gpio-sim
>
> Documentation/admin-guide/gpio/gpio-sim.rst | 72 ++
> Documentation/filesystems/configfs.rst| 6 +-
> drivers/gpio/Kconfig | 8 +
> drivers/gpio/Makefile | 1 +
> drivers/gpio/gpio-sim.c | 874 ++
> fs/configfs/configfs_internal.h | 22 +-
> fs/configfs/dir.c | 245 -
> include/linux/bitmap.h| 127 +--
> include/linux/configfs.h | 3 +-
> lib/bitmap.c | 42 +-
> samples/configfs/configfs_sample.c| 153 +++
> tools/testing/selftests/gpio/.gitignore | 2 +
> tools/testing/selftests/gpio/Makefile | 4 +-
> tools/testing/selftests/gpio/config | 1 +
> tools/testing/selftests/gpio/gpio-chip-info.c | 57 ++
> tools/testing/selftests/gpio/gpio-line-name.c | 55 ++
> tools/testing/selftests/gpio/gpio-sim.sh | 229 +
> 17 files changed, 1815 insertions(+), 86 deletions(-)
> create mode 100644 Documentation/admin-guide/gpio/gpio-sim.rst
> create mode 100644 drivers/gpio/gpio-sim.c
> create mode 100644 tools/testing/selftests/gpio/gpio-chip-info.c
> create mode 100644 tools/testing/selftests/gpio/gpio-line-name.c
> create mode 100755 tools/testing/selftests/gpio/gpio-sim.sh
>
> --
> 2.30.1
>
Hi Joel, Christoph,
FYI The configfs patches from this series have been on the mailing
list for months (long before the GPIO part) and have been re-sent
several times. You have neither acked or opposed these changes. I
don't want to delay the new testing driver anymore so I intend to
apply the entire series and take it upstream through the GPIO tree by
the end of this week.
Best Regards,
Bartosz Golaszewski
[GIT PULL] gpio: fixes for v5.12-rc4
Linus, Please pull the following two fixes for the GPIO subsystem. Both address issues in the core GPIO code. Best Regards, Bartosz Golaszewski The following changes since commit b41ba2ec54a70908067034f139aa23d0dd2985ce: gpiolib: Read "gpio-line-names" from a firmware node (2021-03-08 11:59:17 +0100) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git tags/gpio-fixes-for-v5.12-rc4 for you to fetch changes up to 6cb59afe9e5b45a035bd6b97da6593743feefc72: gpiolib: Assign fwnode to parent's if no primary one provided (2021-03-16 10:18:08 +0100) gpio fixes for v5.12-rc4 - fix the return value in error path in gpiolib_dev_init() - fix the "gpio-line-names" property handling correctly this time Andy Shevchenko (1): gpiolib: Assign fwnode to parent's if no primary one provided Wei Yongjun (1): gpiolib: Fix error return code in gpiolib_dev_init() drivers/gpio/gpiolib.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-)
Re: [PATCH v1 1/1] gpio: mockup: Adjust documentation to the code
On Mon, Mar 15, 2021 at 5:59 PM Andy Shevchenko
wrote:
>
> First of all one of the parameter missed 'mockup' in its name,
> Second, the semantics of the integer pairs depends on the sign
> of the base (the first value in the pair).
>
> Update documentation to reflect the real code behaviour.
>
> Fixes: 2fd1abe99e5f ("Documentation: gpio: add documentation for gpio-mockup")
> Signed-off-by: Andy Shevchenko
> ---
> Documentation/admin-guide/gpio/gpio-mockup.rst | 11 ++-
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/admin-guide/gpio/gpio-mockup.rst
> b/Documentation/admin-guide/gpio/gpio-mockup.rst
> index 9fa1618b3adc..493071da1738 100644
> --- a/Documentation/admin-guide/gpio/gpio-mockup.rst
> +++ b/Documentation/admin-guide/gpio/gpio-mockup.rst
> @@ -17,17 +17,18 @@ module.
> gpio_mockup_ranges
>
> This parameter takes an argument in the form of an array of integer
> -pairs. Each pair defines the base GPIO number (if any) and the number
> -of lines exposed by the chip. If the base GPIO is -1, the gpiolib
> -will assign it automatically.
> +pairs. Each pair defines the base GPIO number (non-negative integer)
> +and the first number after the last of this chip. If the base GPIO
> +is -1, the gpiolib will assign it automatically. while the following
> +parameter is the number of lines exposed by the chip.
>
> -Example: gpio_mockup_ranges=-1,8,-1,16,405,4
> +Example: gpio_mockup_ranges=-1,8,-1,16,405,409
>
> The line above creates three chips. The first one will expose 8
> lines,
> the second 16 and the third 4. The base GPIO for the third chip is
> set
> to 405 while for two first chips it will be assigned automatically.
>
> -gpio_named_lines
> +gpio_mockup_named_lines
>
> This parameter doesn't take any arguments. It lets the driver know
> that
> GPIO lines exposed by it should be named.
> --
> 2.30.2
>
Applied, thanks!
Bartosz
Re: [RESEND PATCH] irq/irq_sim: shrink devm_irq_domain_create_sim()
On Mon, Mar 1, 2021 at 3:27 PM Bartosz Golaszewski wrote:
>
> From: Bartosz Golaszewski
>
> We don't have to use a custom devres structure if all we manage is a
> single pointer - we can use devm_add_action_or_reset() for that and
> shrink the code a bit.
>
> Signed-off-by: Bartosz Golaszewski
> ---
> kernel/irq/irq_sim.c | 31 ---
> 1 file changed, 12 insertions(+), 19 deletions(-)
>
> diff --git a/kernel/irq/irq_sim.c b/kernel/irq/irq_sim.c
> index 48006608baf0..ee40a84641e5 100644
> --- a/kernel/irq/irq_sim.c
> +++ b/kernel/irq/irq_sim.c
> @@ -24,10 +24,6 @@ struct irq_sim_irq_ctx {
> struct irq_sim_work_ctx *work_ctx;
> };
>
> -struct irq_sim_devres {
> - struct irq_domain *domain;
> -};
> -
> static void irq_sim_irqmask(struct irq_data *data)
> {
> struct irq_sim_irq_ctx *irq_ctx = irq_data_get_irq_chip_data(data);
> @@ -216,11 +212,11 @@ void irq_domain_remove_sim(struct irq_domain *domain)
> }
> EXPORT_SYMBOL_GPL(irq_domain_remove_sim);
>
> -static void devm_irq_domain_release_sim(struct device *dev, void *res)
> +static void devm_irq_domain_remove_sim(void *data)
> {
> - struct irq_sim_devres *this = res;
> + struct irq_domain *domain = data;
>
> - irq_domain_remove_sim(this->domain);
> + irq_domain_remove_sim(domain);
> }
>
> /**
> @@ -238,20 +234,17 @@ struct irq_domain *devm_irq_domain_create_sim(struct
> device *dev,
> struct fwnode_handle *fwnode,
> unsigned int num_irqs)
> {
> - struct irq_sim_devres *dr;
> + struct irq_domain *domain;
> + int ret;
>
> - dr = devres_alloc(devm_irq_domain_release_sim,
> - sizeof(*dr), GFP_KERNEL);
> - if (!dr)
> - return ERR_PTR(-ENOMEM);
> + domain = irq_domain_create_sim(fwnode, num_irqs);
> + if (IS_ERR(domain))
> + return domain;
>
> - dr->domain = irq_domain_create_sim(fwnode, num_irqs);
> - if (IS_ERR(dr->domain)) {
> - devres_free(dr);
> - return dr->domain;
> - }
> + ret = devm_add_action_or_reset(dev, devm_irq_domain_remove_sim,
> domain);
> + if (!ret)
> + return ERR_PTR(ret);
>
> - devres_add(dev, dr);
> - return dr->domain;
> + return domain;
> }
> EXPORT_SYMBOL_GPL(devm_irq_domain_create_sim);
> --
> 2.29.1
>
Gentle ping.
Bartosz
Re: [PATCH v1 1/1] gpio: mockup: Drop duplicate NULL check in gpio_mockup_unregister_pdevs()
On Mon, Mar 15, 2021 at 7:51 PM Andy Shevchenko
wrote:
>
> Since platform_device_unregister() is NULL-aware, we don't need to duplicate
> this check. Remove it and fold the rest of the code.
>
> Signed-off-by: Andy Shevchenko
> ---
> drivers/gpio/gpio-mockup.c | 9 ++---
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c
> index 28b757d34046..d7e73876a3b9 100644
> --- a/drivers/gpio/gpio-mockup.c
> +++ b/drivers/gpio/gpio-mockup.c
> @@ -479,15 +479,10 @@ static struct platform_device
> *gpio_mockup_pdevs[GPIO_MOCKUP_MAX_GC];
>
> static void gpio_mockup_unregister_pdevs(void)
> {
> - struct platform_device *pdev;
> int i;
>
> - for (i = 0; i < GPIO_MOCKUP_MAX_GC; i++) {
> - pdev = gpio_mockup_pdevs[i];
> -
> - if (pdev)
> - platform_device_unregister(pdev);
> - }
> + for (i = 0; i < GPIO_MOCKUP_MAX_GC; i++)
> + platform_device_unregister(gpio_mockup_pdevs[i]);
> }
>
> static __init char **gpio_mockup_make_line_names(const char *label,
> --
> 2.30.2
>
Applied, thanks!
Bartosz
Re: [PATCH v1 1/1] gpiolib: Assign fwnode to parent's if no primary one provided
On Mon, Mar 15, 2021 at 7:49 PM Andy Shevchenko
wrote:
>
> On Mon, Mar 15, 2021 at 08:44:30PM +0200, Andy Shevchenko wrote:
> > In case when the properties are supplied in the secondary fwnode
> > (for example, built-in device properties) the fwnode pointer left
> > unassigned. This makes unable to retrieve them.
> >
> > Assign fwnode to parent's if no primary one provided.
>
> Bart, I missed to add you as a Reporter, feel free to do that or tell me I'll
> send a v2.
>
No need, I added myself.
> > Fixes: 7cba1a4d5e16 ("gpiolib: generalize devprop_gpiochip_set_names() for
> > device properties")
> > Fixes: 2afa97e9868f ("gpiolib: Read "gpio-line-names" from a firmware node")
> > Signed-off-by: Andy Shevchenko
>
> Side note: The patch done in this way to avoid conflicts with the future
> (for-next) code once it will be in upstream (v5.12-rcX).
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
Tested-by: Bartosz Golaszewski
Thanks for debugging this! Applied!
Bartosz
Re: [PATCH v1 1/1] gpiolib: Read "gpio-line-names" from a firmware node
On Mon, Mar 15, 2021 at 3:34 PM Andy Shevchenko wrote: > > On Mon, Mar 15, 2021 at 03:04:37PM +0100, Bartosz Golaszewski wrote: > > On Mon, Mar 15, 2021 at 1:50 PM Andy Shevchenko > > wrote: > > > > > > On Mon, Mar 15, 2021 at 12:16:26PM +0200, Andy Shevchenko wrote: > > > > On Mon, Mar 15, 2021 at 10:01:47AM +0100, Bartosz Golaszewski wrote: > > > > > On Fri, Mar 5, 2021 at 1:03 PM Andy Shevchenko > > > > > wrote: > > > > > > > > > Unfortunately while this may fix the particular use-case on STM32, it > > > > > breaks all other users as the 'gpio-line-names' property doesn't live > > > > > on dev_fwnode(>dev) but on dev_fwnode(chip->parent). > > > > > > > > > > How about we first look for this property on the latter and only if > > > > > it's not present descend down to the former fwnode? > > > > > > > > Oops, I have tested on x86 and it worked the same way. > > > > > > > > Lemme check this, but I think the issue rather in ordering when we > > > > apply fwnode > > > > to the newly created device and when we actually retrieve > > > > gpio-line-names > > > > property. > > > > > > Hmm... I can't see how it's possible can be. Can you provide a platform > > > name > > > and pointers to the DTS that has been broken by the change? > > > > > > > I noticed it with gpio-mockup (libgpiod tests failed on v5.12-rc3) and > > the WiP gpio-sim - but it's the same on most DT platforms. The node > > that contains the `gpio-line-names` is the one associated with the > > platform device passed to the GPIO driver. The gpiolib then creates > > another struct device that becomes the child of that node but it > > doesn't copy the parent's properties to it (nor should it). > > > > Every driver that reads device properties does it from the parent > > device, not the one in gdev - whether it uses of_, fwnode_ or generic > > device_ properties. > > What you are telling contradicts with the idea of copying parent's fwnode > (or OF node) in the current code. > Ha! While the OF node of the parent device is indeed assigned to the gdev's dev, the same isn't done in the core code for fwnodes and simulated chips don't have an associated OF node, so this is the culprit I suppose. Bart > Basically to access the properties we have to use either what specific driver > supplied (by setting gpiochip->of_node or by leaving it NULL and in this case > gpiochip_add_data_with_key() will copy it from the parent. > > That said, we shouldn't care about parent vs. GPIO device fwnode when reading > properties. So, bug is somewhere else. > > In any case, I will test with the gpio-mockup, thanks! > > -- > With Best Regards, > Andy Shevchenko > >
Re: [PATCH 5.10 081/290] gpiolib: Read "gpio-line-names" from a firmware node
On Mon, Mar 15, 2021 at 4:01 PM Marek Vasut wrote: > > On 3/15/21 2:52 PM, gre...@linuxfoundation.org wrote: > > From: Greg Kroah-Hartman > > > > From: Andy Shevchenko > > > > commit b41ba2ec54a70908067034f139aa23d0dd2985ce upstream. > > > > On STM32MP1, the GPIO banks are subnodes of pin-controller@50002000, > > see arch/arm/boot/dts/stm32mp151.dtsi. The driver for > > pin-controller@50002000 is in drivers/pinctrl/stm32/pinctrl-stm32.c > > and iterates over all of its DT subnodes when registering each GPIO > > bank gpiochip. Each gpiochip has: > > > >- gpio_chip.parent = dev, > > where dev is the device node of the pin controller > >- gpio_chip.of_node = np, > > which is the OF node of the GPIO bank > > > > Therefore, dev_fwnode(chip->parent) != of_fwnode_handle(chip.of_node), > > i.e. pin-controller@50002000 != pin-controller@50002000/gpio@5000*000. > > > > The original code behaved correctly, as it extracted the "gpio-line-names" > > from of_fwnode_handle(chip.of_node) = pin-controller@50002000/gpio@5000*000. > > > > To achieve the same behaviour, read property from the firmware node. > > There seem to be some discussion going on around this patch, so please > postpone backporting until that is settled. Same for v5.11 backport. I > hope Andy/Bartosz agrees ? Yes, this patch broke at least the testing module and we're working to determine if it breaks DT drivers too. Bart
Re: [PATCH v1 1/1] gpiolib: Read "gpio-line-names" from a firmware node
On Mon, Mar 15, 2021 at 1:50 PM Andy Shevchenko wrote: > > On Mon, Mar 15, 2021 at 12:16:26PM +0200, Andy Shevchenko wrote: > > On Mon, Mar 15, 2021 at 10:01:47AM +0100, Bartosz Golaszewski wrote: > > > On Fri, Mar 5, 2021 at 1:03 PM Andy Shevchenko > > > wrote: > > > > > Unfortunately while this may fix the particular use-case on STM32, it > > > breaks all other users as the 'gpio-line-names' property doesn't live > > > on dev_fwnode(>dev) but on dev_fwnode(chip->parent). > > > > > > How about we first look for this property on the latter and only if > > > it's not present descend down to the former fwnode? > > > > Oops, I have tested on x86 and it worked the same way. > > > > Lemme check this, but I think the issue rather in ordering when we apply > > fwnode > > to the newly created device and when we actually retrieve gpio-line-names > > property. > > Hmm... I can't see how it's possible can be. Can you provide a platform name > and pointers to the DTS that has been broken by the change? > I noticed it with gpio-mockup (libgpiod tests failed on v5.12-rc3) and the WiP gpio-sim - but it's the same on most DT platforms. The node that contains the `gpio-line-names` is the one associated with the platform device passed to the GPIO driver. The gpiolib then creates another struct device that becomes the child of that node but it doesn't copy the parent's properties to it (nor should it). Every driver that reads device properties does it from the parent device, not the one in gdev - whether it uses of_, fwnode_ or generic device_ properties. Bartosz
[PATCH v5 10/11] selftests: gpio: add a helper for reading GPIO line names
From: Bartosz Golaszewski
Add a simple program that allows to read GPIO line names from the
character device. This will be used in gpio-sim selftests.
Signed-off-by: Bartosz Golaszewski
---
tools/testing/selftests/gpio/.gitignore | 1 +
tools/testing/selftests/gpio/Makefile | 2 +-
tools/testing/selftests/gpio/gpio-line-name.c | 55 +++
3 files changed, 57 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/gpio/gpio-line-name.c
diff --git a/tools/testing/selftests/gpio/.gitignore
b/tools/testing/selftests/gpio/.gitignore
index 4ea4f58dab1a..ededb077a3a6 100644
--- a/tools/testing/selftests/gpio/.gitignore
+++ b/tools/testing/selftests/gpio/.gitignore
@@ -1,3 +1,4 @@
# SPDX-License-Identifier: GPL-2.0-only
gpio-mockup-cdev
gpio-chip-info
+gpio-line-name
diff --git a/tools/testing/selftests/gpio/Makefile
b/tools/testing/selftests/gpio/Makefile
index 84b48547f94c..d7d8f1985d99 100644
--- a/tools/testing/selftests/gpio/Makefile
+++ b/tools/testing/selftests/gpio/Makefile
@@ -2,6 +2,6 @@
TEST_PROGS := gpio-mockup.sh
TEST_FILES := gpio-mockup-sysfs.sh
-TEST_GEN_PROGS_EXTENDED := gpio-mockup-cdev gpio-chip-info
+TEST_GEN_PROGS_EXTENDED := gpio-mockup-cdev gpio-chip-info gpio-line-name
include ../lib.mk
diff --git a/tools/testing/selftests/gpio/gpio-line-name.c
b/tools/testing/selftests/gpio/gpio-line-name.c
new file mode 100644
index ..a52e75bc37ba
--- /dev/null
+++ b/tools/testing/selftests/gpio/gpio-line-name.c
@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * GPIO character device helper for reading line names.
+ *
+ * Copyright (C) 2021 Bartosz Golaszewski
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+static void print_usage(void)
+{
+ printf("usage:\n");
+ printf(" gpio-line-name \n");
+}
+
+int main(int argc, char **argv)
+{
+ struct gpio_v2_line_info info;
+ int fd, ret;
+ char *endp;
+
+ if (argc != 3) {
+ print_usage();
+ return EXIT_FAILURE;
+ }
+
+ fd = open(argv[1], O_RDWR);
+ if (fd < 0) {
+ perror("unable to open the GPIO chip");
+ return EXIT_FAILURE;
+ }
+
+ memset(, 0, sizeof(info));
+ info.offset = strtoul(argv[2], , 10);
+ if (*endp != '\0') {
+ print_usage();
+ return EXIT_FAILURE;
+ }
+
+ ret = ioctl(fd, GPIO_V2_GET_LINEINFO_IOCTL, );
+ if (ret) {
+ perror("line info ioctl failed");
+ return EXIT_FAILURE;
+ }
+
+ printf("%s\n", info.name);
+
+ return EXIT_SUCCESS;
+}
--
2.30.1
[PATCH v5 11/11] selftests: gpio: add test cases for gpio-sim
From: Bartosz Golaszewski
Add a set of tests for the new gpio-sim module. This is a pure shell
test-suite and uses the helper programs available in the gpio selftests
directory. These test-cases only test the functionalities exposed by the
gpio-sim driver, not those handled by core gpiolib code.
Signed-off-by: Bartosz Golaszewski
---
tools/testing/selftests/gpio/Makefile| 2 +-
tools/testing/selftests/gpio/config | 1 +
tools/testing/selftests/gpio/gpio-sim.sh | 229 +++
3 files changed, 231 insertions(+), 1 deletion(-)
create mode 100755 tools/testing/selftests/gpio/gpio-sim.sh
diff --git a/tools/testing/selftests/gpio/Makefile
b/tools/testing/selftests/gpio/Makefile
index d7d8f1985d99..4c6df61c76a8 100644
--- a/tools/testing/selftests/gpio/Makefile
+++ b/tools/testing/selftests/gpio/Makefile
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
-TEST_PROGS := gpio-mockup.sh
+TEST_PROGS := gpio-mockup.sh gpio-sim.sh
TEST_FILES := gpio-mockup-sysfs.sh
TEST_GEN_PROGS_EXTENDED := gpio-mockup-cdev gpio-chip-info gpio-line-name
diff --git a/tools/testing/selftests/gpio/config
b/tools/testing/selftests/gpio/config
index ce100342c20b..409a8532facc 100644
--- a/tools/testing/selftests/gpio/config
+++ b/tools/testing/selftests/gpio/config
@@ -1,3 +1,4 @@
CONFIG_GPIOLIB=y
CONFIG_GPIO_CDEV=y
CONFIG_GPIO_MOCKUP=m
+CONFIG_GPIO_SIM=m
diff --git a/tools/testing/selftests/gpio/gpio-sim.sh
b/tools/testing/selftests/gpio/gpio-sim.sh
new file mode 100755
index ..fcca6ec611f8
--- /dev/null
+++ b/tools/testing/selftests/gpio/gpio-sim.sh
@@ -0,0 +1,229 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2021 Bartosz Golaszewski
+
+BASE_DIR=`dirname $0`
+CONFIGFS_DIR="/sys/kernel/config/gpio-sim"
+PENDING_DIR=$CONFIGFS_DIR/pending
+LIVE_DIR=$CONFIGFS_DIR/live
+MODULE="gpio-sim"
+
+fail() {
+ echo "$*" >&2
+ echo "GPIO $MODULE test FAIL"
+ exit 1
+}
+
+skip() {
+ echo "$*" >&2
+ echo "GPIO $MODULE test SKIP"
+ exit 4
+}
+
+configfs_cleanup() {
+ for DIR in `ls $LIVE_DIR`; do
+ mv $LIVE_DIR/$DIR $PENDING_DIR
+ done
+
+ for DIR in `ls $PENDING_DIR`; do
+ rmdir $PENDING_DIR/$DIR
+ done
+}
+
+create_pending_chip() {
+ local NAME="$1"
+ local LABEL="$2"
+ local NUM_LINES="$3"
+ local LINE_NAMES="$4"
+ local CHIP_DIR="$PENDING_DIR/$NAME"
+
+ mkdir $CHIP_DIR
+ test -n "$LABEL" && echo $LABEL > $CHIP_DIR/label
+ test -n "$NUM_LINES" && echo $NUM_LINES > $CHIP_DIR/num_lines
+ if [ -n "$LINE_NAMES" ]; then
+ echo $LINE_NAMES 2> /dev/null > $CHIP_DIR/line_names
+ # This one can fail
+ if [ "$?" -ne "0" ]; then
+ return 1
+ fi
+ fi
+}
+
+create_live_chip() {
+ local CHIP_DIR="$PENDING_DIR/$1"
+
+ create_pending_chip "$@" || fail "unable to create the chip configfs
item"
+ mv $CHIP_DIR $LIVE_DIR || fail "unable to commit the chip configfs item"
+}
+
+remove_pending_chip() {
+ local NAME="$1"
+
+ rmdir $PENDING_DIR/$NAME || fail "unable to remove the chip configfs
item"
+}
+
+remove_live_chip() {
+ local NAME="$1"
+
+ mv $LIVE_DIR/$NAME $PENDING_DIR || fail "unable to uncommit the chip
configfs item"
+ remove_pending_chip "$@"
+}
+
+configfs_chip_name() {
+ local CHIP="$1"
+
+ cat $LIVE_DIR/$CHIP/chip_name 2> /dev/null || return 1
+}
+
+configfs_dev_name() {
+ local CHIP="$1"
+
+ cat $LIVE_DIR/$CHIP/dev_name 2> /dev/null || return 1
+}
+
+get_chip_num_lines() {
+ local CHIP="$1"
+
+ $BASE_DIR/gpio-chip-info /dev/`configfs_chip_name $CHIP` num-lines
+}
+
+get_chip_label() {
+ local CHIP="$1"
+
+ $BASE_DIR/gpio-chip-info /dev/`configfs_chip_name $CHIP` label
+}
+
+get_line_name() {
+ local CHIP="$1"
+ local OFFSET="$2"
+
+ $BASE_DIR/gpio-line-name /dev/`configfs_chip_name $CHIP` $OFFSET
+}
+
+sysfs_set_pull() {
+ local CHIP="$1"
+ local OFFSET="$2"
+ local PULL="$3"
+ local SYSFSPATH="/sys/devices/platform/`configfs_dev_name
$CHIP`/line-ctrl/gpio$OFFSET"
+
+ echo $PULL > $SYSFSPATH
+}
+
+# Load the gpio-sim module. This will pull in configfs if needed too.
+modprobe gpio-sim || skip "unable to load the gpio-sim module"
+# Make sure configfs is mounted at /sys/kernel/config. Wait a bit if needed.
+for IDX in `seq 5`; do
+ if [ "$IDX" -eq "5" ]; then
+ skip
[PATCH v5 05/11] lib: bitmap: remove the 'extern' keyword from function declarations
From: Bartosz Golaszewski The 'extern' keyword doesn't have any benefits in header files. Remove it. Signed-off-by: Bartosz Golaszewski Reviewed-by: Andy Shevchenko --- include/linux/bitmap.h | 115 - 1 file changed, 57 insertions(+), 58 deletions(-) diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h index 70a932470b2d..6939a8983026 100644 --- a/include/linux/bitmap.h +++ b/include/linux/bitmap.h @@ -118,54 +118,53 @@ * Allocation and deallocation of bitmap. * Provided in lib/bitmap.c to avoid circular dependency. */ -extern unsigned long *bitmap_alloc(unsigned int nbits, gfp_t flags); -extern unsigned long *bitmap_zalloc(unsigned int nbits, gfp_t flags); -extern void bitmap_free(const unsigned long *bitmap); +unsigned long *bitmap_alloc(unsigned int nbits, gfp_t flags); +unsigned long *bitmap_zalloc(unsigned int nbits, gfp_t flags); +void bitmap_free(const unsigned long *bitmap); /* * lib/bitmap.c provides these functions: */ -extern int __bitmap_equal(const unsigned long *bitmap1, - const unsigned long *bitmap2, unsigned int nbits); -extern bool __pure __bitmap_or_equal(const unsigned long *src1, -const unsigned long *src2, -const unsigned long *src3, -unsigned int nbits); -extern void __bitmap_complement(unsigned long *dst, const unsigned long *src, - unsigned int nbits); -extern void __bitmap_shift_right(unsigned long *dst, const unsigned long *src, - unsigned int shift, unsigned int nbits); -extern void __bitmap_shift_left(unsigned long *dst, const unsigned long *src, - unsigned int shift, unsigned int nbits); -extern void bitmap_cut(unsigned long *dst, const unsigned long *src, - unsigned int first, unsigned int cut, - unsigned int nbits); -extern int __bitmap_and(unsigned long *dst, const unsigned long *bitmap1, +int __bitmap_equal(const unsigned long *bitmap1, + const unsigned long *bitmap2, unsigned int nbits); +bool __pure __bitmap_or_equal(const unsigned long *src1, + const unsigned long *src2, + const unsigned long *src3, + unsigned int nbits); +void __bitmap_complement(unsigned long *dst, const unsigned long *src, +unsigned int nbits); +void __bitmap_shift_right(unsigned long *dst, const unsigned long *src, + unsigned int shift, unsigned int nbits); +void __bitmap_shift_left(unsigned long *dst, const unsigned long *src, +unsigned int shift, unsigned int nbits); +void bitmap_cut(unsigned long *dst, const unsigned long *src, + unsigned int first, unsigned int cut, unsigned int nbits); +int __bitmap_and(unsigned long *dst, const unsigned long *bitmap1, +const unsigned long *bitmap2, unsigned int nbits); +void __bitmap_or(unsigned long *dst, const unsigned long *bitmap1, +const unsigned long *bitmap2, unsigned int nbits); +void __bitmap_xor(unsigned long *dst, const unsigned long *bitmap1, + const unsigned long *bitmap2, unsigned int nbits); +int __bitmap_andnot(unsigned long *dst, const unsigned long *bitmap1, + const unsigned long *bitmap2, unsigned int nbits); +void __bitmap_replace(unsigned long *dst, + const unsigned long *old, const unsigned long *new, + const unsigned long *mask, unsigned int nbits); +int __bitmap_intersects(const unsigned long *bitmap1, const unsigned long *bitmap2, unsigned int nbits); -extern void __bitmap_or(unsigned long *dst, const unsigned long *bitmap1, - const unsigned long *bitmap2, unsigned int nbits); -extern void __bitmap_xor(unsigned long *dst, const unsigned long *bitmap1, - const unsigned long *bitmap2, unsigned int nbits); -extern int __bitmap_andnot(unsigned long *dst, const unsigned long *bitmap1, - const unsigned long *bitmap2, unsigned int nbits); -extern void __bitmap_replace(unsigned long *dst, - const unsigned long *old, const unsigned long *new, - const unsigned long *mask, unsigned int nbits); -extern int __bitmap_intersects(const unsigned long *bitmap1, - const unsigned long *bitmap2, unsigned int nbits); -extern int __bitmap_subset(const unsigned long *bitmap1, - const unsigned long *bitmap2, unsigned int nbits); -extern int __bitmap_weight(const unsigned long *bitmap, unsigned int nbits); -extern void __bitmap_set(unsigned long *map, unsigned int start, int len); -extern void __bitmap_clear(unsigned long *map, unsigned int start, int len); - -extern unsigned
[PATCH v5 08/11] gpio: sim: new testing module
From: Bartosz Golaszewski Implement a new, modern GPIO testing module controlled by configfs attributes instead of module parameters. The goal of this driver is to provide a replacement for gpio-mockup that will be easily extensible with new features and doesn't require reloading the module to change the setup. Signed-off-by: Bartosz Golaszewski Reviewed-by: Andy Shevchenko Reviewed-by: Linus Walleij --- Documentation/admin-guide/gpio/gpio-sim.rst | 72 ++ drivers/gpio/Kconfig| 8 + drivers/gpio/Makefile | 1 + drivers/gpio/gpio-sim.c | 874 4 files changed, 955 insertions(+) create mode 100644 Documentation/admin-guide/gpio/gpio-sim.rst create mode 100644 drivers/gpio/gpio-sim.c diff --git a/Documentation/admin-guide/gpio/gpio-sim.rst b/Documentation/admin-guide/gpio/gpio-sim.rst new file mode 100644 index ..08eac487e35e --- /dev/null +++ b/Documentation/admin-guide/gpio/gpio-sim.rst @@ -0,0 +1,72 @@ +.. SPDX-License-Identifier: GPL-2.0-or-later + +Configfs GPIO Simulator +=== + +The configfs GPIO Simulator (gpio-sim) provides a way to create simulated GPIO +chips for testing purposes. The lines exposed by these chips can be accessed +using the standard GPIO character device interface as well as manipulated +using sysfs attributes. + +Creating simulated chips + + +The gpio-sim module registers a configfs subsystem called 'gpio-sim'. It's a +subsystem with committable items which means two subdirectories are created in +the filesystem: pending and live. For more information on configfs and +committable items, please refer to Documentation/filesystems/configfs.rst. + +In order to instantiate a new simulated chip, the user needs to mkdir() a new +directory in pending/. Inside each new directory, there's a set of attributes +that can be used to configure the new chip. Once the configuration is complete, +the user needs to use rename() to move the chip to the live/ directory. This +creates and registers the new device. + +In order to destroy a simulated chip, it has to be moved back to pending first +and then removed using rmdir(). + +Currently supported configuration attributes are: + + num_lines - an unsigned integer value defining the number of GPIO lines to + export + + label - a string defining the label for the GPIO chip + + line_names - a list of GPIO line names in the form of quoted strings + separated by commas, e.g.: '"foo", "bar", "", "foobar"'. The + number of strings doesn't have to be equal to the value set in + the num_lines attribute. If it's lower than the number of lines, + the remaining lines are unnamed. If it's larger, the superfluous + lines are ignored. A name of the form: '""' means the line + should be unnamed. + +Additionally two read-only attributes named 'chip_name' and 'dev_name' are +exposed in order to provide users with a mapping from configfs directories to +the actual devices created in the kernel. The former returns the name of the +GPIO device as assigned by gpiolib (i.e. "gpiochip0", "gpiochip1", etc.). The +latter returns the parent device name as defined by the gpio-sim driver (i.e. +"gpio-sim.0", "gpio-sim.1", etc.). This allows user-space to map the configfs +items both to the correct character device file as well as the associated entry +in sysfs. + +Simulated GPIO chips can also be defined in device-tree. The compatible string +must be: "gpio-simulator". Supported properties are: + + "gpio-sim,label" - chip label + + "gpio-sim,nr-gpios" - number of lines + +Other standard GPIO properties (like "gpio-line-names" and gpio-hog) are also +supported. + +Manipulating simulated lines + + +Each simulated GPIO chip creates a sysfs attribute group under its device +directory called 'line-ctrl'. Inside each group, there's a separate attribute +for each GPIO line. The name of the attribute is of the form 'gpioX' where X +is the line's offset in the chip. + +Reading from a line attribute returns the current value. Writing to it (0 or 1) +changes the configuration of the simulated pull-up/pull-down resistor +(1 - pull-up, 0 - pull-down). diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index e3607ec4c2e8..b6b6150d0d04 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -1633,6 +1633,14 @@ config GPIO_MOCKUP tools/testing/selftests/gpio/gpio-mockup.sh. Reference the usage in it. +config GPIO_SIM + tristate "GPIO Simulator Module" + select IRQ_SIM + select CONFIGFS_FS + help + This enables the GPIO simulator - a configfs-based GPIO testing + driver. + endmenu endif diff
[PATCH v5 06/11] lib: bitmap: order includes alphabetically
From: Bartosz Golaszewski For better readability and maintenance: order the includes in bitmap source files alphabetically. Signed-off-by: Bartosz Golaszewski Reviewed-by: Andy Shevchenko --- include/linux/bitmap.h | 4 ++-- lib/bitmap.c | 9 + 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h index 6939a8983026..3282db97e06c 100644 --- a/include/linux/bitmap.h +++ b/include/linux/bitmap.h @@ -4,10 +4,10 @@ #ifndef __ASSEMBLY__ -#include #include -#include #include +#include +#include /* * bitmaps provide bit arrays that consume one or more unsigned diff --git a/lib/bitmap.c b/lib/bitmap.c index 75006c4036e9..78f70d9007ad 100644 --- a/lib/bitmap.c +++ b/lib/bitmap.c @@ -3,17 +3,18 @@ * lib/bitmap.c * Helper functions for bitmap.h. */ -#include -#include -#include -#include + #include #include #include +#include +#include +#include #include #include #include #include +#include #include #include -- 2.30.1
[PATCH v5 09/11] selftests: gpio: provide a helper for reading chip info
From: Bartosz Golaszewski
Add a simple program that allows to retrieve chip properties from the
GPIO character device. This will be used in gpio-sim selftests.
Signed-off-by: Bartosz Golaszewski
---
tools/testing/selftests/gpio/.gitignore | 1 +
tools/testing/selftests/gpio/Makefile | 2 +-
tools/testing/selftests/gpio/gpio-chip-info.c | 57 +++
3 files changed, 59 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/gpio/gpio-chip-info.c
diff --git a/tools/testing/selftests/gpio/.gitignore
b/tools/testing/selftests/gpio/.gitignore
index a4969f7ee020..4ea4f58dab1a 100644
--- a/tools/testing/selftests/gpio/.gitignore
+++ b/tools/testing/selftests/gpio/.gitignore
@@ -1,2 +1,3 @@
# SPDX-License-Identifier: GPL-2.0-only
gpio-mockup-cdev
+gpio-chip-info
diff --git a/tools/testing/selftests/gpio/Makefile
b/tools/testing/selftests/gpio/Makefile
index 39f2bbe8dd3d..84b48547f94c 100644
--- a/tools/testing/selftests/gpio/Makefile
+++ b/tools/testing/selftests/gpio/Makefile
@@ -2,6 +2,6 @@
TEST_PROGS := gpio-mockup.sh
TEST_FILES := gpio-mockup-sysfs.sh
-TEST_GEN_PROGS_EXTENDED := gpio-mockup-cdev
+TEST_GEN_PROGS_EXTENDED := gpio-mockup-cdev gpio-chip-info
include ../lib.mk
diff --git a/tools/testing/selftests/gpio/gpio-chip-info.c
b/tools/testing/selftests/gpio/gpio-chip-info.c
new file mode 100644
index ..4d26fa7c254a
--- /dev/null
+++ b/tools/testing/selftests/gpio/gpio-chip-info.c
@@ -0,0 +1,57 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * GPIO character device helper for reading chip information.
+ *
+ * Copyright (C) 2021 Bartosz Golaszewski
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+static void print_usage(void)
+{
+ printf("usage:\n");
+ printf(" gpio-chip-info [name|label|num-lines]\n");
+}
+
+int main(int argc, char **argv)
+{
+ struct gpiochip_info info;
+ int fd, ret;
+
+ if (argc !=3) {
+ print_usage();
+ return EXIT_FAILURE;
+ }
+
+ fd = open(argv[1], O_RDWR);
+ if (fd < 0) {
+ perror("unable to open the GPIO chip");
+ return EXIT_FAILURE;
+ }
+
+ memset(, 0, sizeof(info));
+ ret = ioctl(fd, GPIO_GET_CHIPINFO_IOCTL, );
+ if (ret) {
+ perror("chip info ioctl failed");
+ return EXIT_FAILURE;
+ }
+
+ if (strcmp(argv[2], "name") == 0) {
+ printf("%s\n", info.name);
+ } else if (strcmp(argv[2], "label") == 0) {
+ printf("%s\n", info.label);
+ } else if (strcmp(argv[2], "num-lines") == 0) {
+ printf("%u\n", info.lines);
+ } else {
+ fprintf(stderr, "unknown command: %s\n", argv[2]);
+ return EXIT_FAILURE;
+ }
+
+ return EXIT_SUCCESS;
+}
--
2.30.1
[PATCH v5 07/11] lib: bitmap: provide devm_bitmap_alloc() and devm_bitmap_zalloc()
From: Bartosz Golaszewski
Provide managed variants of bitmap_alloc() and bitmap_zalloc().
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Andy Shevchenko
---
include/linux/bitmap.h | 8
lib/bitmap.c | 33 +
2 files changed, 41 insertions(+)
diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index 3282db97e06c..73d039476fa4 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -9,6 +9,8 @@
#include
#include
+struct device;
+
/*
* bitmaps provide bit arrays that consume one or more unsigned
* longs. The bitmap interface and available operations are listed
@@ -122,6 +124,12 @@ unsigned long *bitmap_alloc(unsigned int nbits, gfp_t
flags);
unsigned long *bitmap_zalloc(unsigned int nbits, gfp_t flags);
void bitmap_free(const unsigned long *bitmap);
+/* Managed variants of the above. */
+unsigned long *devm_bitmap_alloc(struct device *dev,
+unsigned int nbits, gfp_t flags);
+unsigned long *devm_bitmap_zalloc(struct device *dev,
+ unsigned int nbits, gfp_t flags);
+
/*
* lib/bitmap.c provides these functions:
*/
diff --git a/lib/bitmap.c b/lib/bitmap.c
index 78f70d9007ad..27e08c0e547e 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -8,6 +8,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -1263,6 +1264,38 @@ void bitmap_free(const unsigned long *bitmap)
}
EXPORT_SYMBOL(bitmap_free);
+static void devm_bitmap_free(void *data)
+{
+ unsigned long *bitmap = data;
+
+ bitmap_free(bitmap);
+}
+
+unsigned long *devm_bitmap_alloc(struct device *dev,
+unsigned int nbits, gfp_t flags)
+{
+ unsigned long *bitmap;
+ int ret;
+
+ bitmap = bitmap_alloc(nbits, flags);
+ if (!bitmap)
+ return NULL;
+
+ ret = devm_add_action_or_reset(dev, devm_bitmap_free, bitmap);
+ if (ret)
+ return NULL;
+
+ return bitmap;
+}
+EXPORT_SYMBOL_GPL(devm_bitmap_alloc);
+
+unsigned long *devm_bitmap_zalloc(struct device *dev,
+ unsigned int nbits, gfp_t flags)
+{
+ return devm_bitmap_alloc(dev, nbits, flags | __GFP_ZERO);
+}
+EXPORT_SYMBOL_GPL(devm_bitmap_zalloc);
+
#if BITS_PER_LONG == 64
/**
* bitmap_from_arr32 - copy the contents of u32 array of bits to bitmap
--
2.30.1
[PATCH v5 02/11] configfs: use (1UL << bit) for internal flags
From: Bartosz Golaszewski
For better readability and maintenance: use the (1UL << bit) for flag
definitions.
Signed-off-by: Bartosz Golaszewski
Acked-by: Linus Walleij
---
fs/configfs/configfs_internal.h | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/fs/configfs/configfs_internal.h b/fs/configfs/configfs_internal.h
index 9a3aed249692..b495c9f043d4 100644
--- a/fs/configfs/configfs_internal.h
+++ b/fs/configfs/configfs_internal.h
@@ -46,16 +46,16 @@ struct configfs_dirent {
struct configfs_fragment *s_frag;
};
-#define CONFIGFS_ROOT 0x0001
-#define CONFIGFS_DIR 0x0002
-#define CONFIGFS_ITEM_ATTR 0x0004
-#define CONFIGFS_ITEM_BIN_ATTR 0x0008
-#define CONFIGFS_ITEM_LINK 0x0020
-#define CONFIGFS_USET_DIR 0x0040
-#define CONFIGFS_USET_DEFAULT 0x0080
-#define CONFIGFS_USET_DROPPING 0x0100
-#define CONFIGFS_USET_IN_MKDIR 0x0200
-#define CONFIGFS_USET_CREATING 0x0400
+#define CONFIGFS_ROOT (1UL << 0)
+#define CONFIGFS_DIR (1UL << 1)
+#define CONFIGFS_ITEM_ATTR (1UL << 2)
+#define CONFIGFS_ITEM_BIN_ATTR (1UL << 3)
+#define CONFIGFS_ITEM_LINK (1UL << 5)
+#define CONFIGFS_USET_DIR (1UL << 6)
+#define CONFIGFS_USET_DEFAULT (1UL << 7)
+#define CONFIGFS_USET_DROPPING (1UL << 8)
+#define CONFIGFS_USET_IN_MKDIR (1UL << 9)
+#define CONFIGFS_USET_CREATING (1UL << 10)
#define CONFIGFS_NOT_PINNED(CONFIGFS_ITEM_ATTR | CONFIGFS_ITEM_BIN_ATTR)
extern struct mutex configfs_symlink_mutex;
--
2.30.1
[PATCH v5 01/11] configfs: increase the item name length
From: Bartosz Golaszewski 20 characters limit for item name is relatively small. Let's increase it to 32 to fit '04-committable-children' - a name we'll use in the sample code for committable items. Signed-off-by: Bartosz Golaszewski Acked-by: Linus Walleij --- include/linux/configfs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/configfs.h b/include/linux/configfs.h index 2e8c69b43c64..4f76dcc08134 100644 --- a/include/linux/configfs.h +++ b/include/linux/configfs.h @@ -27,7 +27,7 @@ #include/* struct kref */ #include /* struct mutex */ -#define CONFIGFS_ITEM_NAME_LEN 20 +#define CONFIGFS_ITEM_NAME_LEN 32 struct module; -- 2.30.1
[PATCH v5 00/11] gpio: implement the configfs testing module
From: Bartosz Golaszewski
This series adds a new GPIO testing module based on configfs committable items
and sysfs. The goal is to provide a testing driver that will be configurable
at runtime (won't need module reload) and easily extensible. The control over
the attributes is also much more fine-grained than in gpio-mockup.
This series also contains a respin of the patches I sent separately to the
configfs maintainers - these patches implement the concept of committable
items that was well defined for a long time but never actually completed.
Apart from the new driver itself, its selftests and the configfs patches, this
series contains some changes to the bitmap API - most importantly: it adds
devres managed variants of bitmap_alloc() and bitmap_zalloc().
v1 -> v2:
- add selftests for gpio-sim
- add helper programs for selftests
- update the configfs rename callback to work with the new API introduced in
v5.11
- fix a missing quote in the documentation
- use !! whenever using bits operation that are required to return 0 or 1
- use provided bitmap API instead of reimplementing copy or fill operations
- fix a deadlock in gpio_sim_direction_output()
- add new read-only configfs attributes for mapping of configfs items to GPIO
device names
- and address other minor issues pointed out in reviews of v1
v2 -> v3:
- use devm_bitmap_alloc() instead of the zalloc variant if we're initializing
the bitmap with 1s
- drop the patch exporting device_is_bound()
- don't return -ENODEV from dev_nam and chip_name configfs attributes, return
a string indicating that the device is not available yet ('n/a')
- fix indentation where it makes sense
- don't protect IDA functions which use their own locking and where it's not
needed
- use kmemdup() instead of kzalloc() + memcpy()
- collected review tags
- minor coding style fixes
v3 -> v4:
- return 'none' instead of 'n/a' from dev_name and chip_name before the device
is registered
- use sysfs_emit() instead of s*printf()
- drop GPIO_SIM_MAX_PROP as it's only used in an array's definition where it's
fine to hardcode the value
v4 -> v5:
- export devm bitmap functions with EXPORT_SYMBOL_GPL() instead of a simple
EXPORT_SYMBOL()
Bartosz Golaszewski (11):
configfs: increase the item name length
configfs: use (1UL << bit) for internal flags
configfs: implement committable items
samples: configfs: add a committable group
lib: bitmap: remove the 'extern' keyword from function declarations
lib: bitmap: order includes alphabetically
lib: bitmap: provide devm_bitmap_alloc() and devm_bitmap_zalloc()
gpio: sim: new testing module
selftests: gpio: provide a helper for reading chip info
selftests: gpio: add a helper for reading GPIO line names
selftests: gpio: add test cases for gpio-sim
Documentation/admin-guide/gpio/gpio-sim.rst | 72 ++
Documentation/filesystems/configfs.rst| 6 +-
drivers/gpio/Kconfig | 8 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-sim.c | 874 ++
fs/configfs/configfs_internal.h | 22 +-
fs/configfs/dir.c | 245 -
include/linux/bitmap.h| 127 +--
include/linux/configfs.h | 3 +-
lib/bitmap.c | 42 +-
samples/configfs/configfs_sample.c| 153 +++
tools/testing/selftests/gpio/.gitignore | 2 +
tools/testing/selftests/gpio/Makefile | 4 +-
tools/testing/selftests/gpio/config | 1 +
tools/testing/selftests/gpio/gpio-chip-info.c | 57 ++
tools/testing/selftests/gpio/gpio-line-name.c | 55 ++
tools/testing/selftests/gpio/gpio-sim.sh | 229 +
17 files changed, 1815 insertions(+), 86 deletions(-)
create mode 100644 Documentation/admin-guide/gpio/gpio-sim.rst
create mode 100644 drivers/gpio/gpio-sim.c
create mode 100644 tools/testing/selftests/gpio/gpio-chip-info.c
create mode 100644 tools/testing/selftests/gpio/gpio-line-name.c
create mode 100755 tools/testing/selftests/gpio/gpio-sim.sh
--
2.30.1
[PATCH v5 04/11] samples: configfs: add a committable group
From: Bartosz Golaszewski
Add an example of using committable items to configfs samples. Each
config item has two attributes: read-write 'storeme' which works
similarly to other examples in this file and a read-only 'committed'
attribute which changes its value between false and true depending on
whether it's committed or not at the moment.
Signed-off-by: Bartosz Golaszewski
Acked-by: Linus Walleij
---
samples/configfs/configfs_sample.c | 153 +
1 file changed, 153 insertions(+)
diff --git a/samples/configfs/configfs_sample.c
b/samples/configfs/configfs_sample.c
index f9008be7a8a1..9bef74e4369d 100644
--- a/samples/configfs/configfs_sample.c
+++ b/samples/configfs/configfs_sample.c
@@ -315,6 +315,158 @@ static struct configfs_subsystem group_children_subsys = {
/* - */
+/*
+ * 04-committable-children
+ *
+ * This is an example of a committable group. It's similar to the simple
+ * children example but each config_item has an additional 'committed'
+ * attribute which is read-only and is only modified when the config_item
+ * is moved from the 'pending' to the 'live' directory.
+ */
+
+struct committable_child {
+ struct config_item item;
+ int storeme;
+ bool committed;
+};
+
+static inline struct committable_child *
+to_committable_child(struct config_item *item)
+{
+ return container_of(item, struct committable_child, item);
+}
+
+static ssize_t
+committable_child_storeme_show(struct config_item *item, char *page)
+{
+ return sprintf(page, "%d\n", to_committable_child(item)->storeme);
+}
+
+static ssize_t committable_child_storeme_store(struct config_item *item,
+ const char *page, size_t count)
+{
+ struct committable_child *child = to_committable_child(item);
+ int ret;
+
+ if (child->committed)
+ return -EPERM;
+
+ ret = kstrtoint(page, 10, >storeme);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+CONFIGFS_ATTR(committable_child_, storeme);
+
+static ssize_t
+committable_child_committed_show(struct config_item *item, char *page)
+{
+ return sprintf(page, "%s\n",
+ to_committable_child(item)->committed ? "true" : "false");
+}
+
+CONFIGFS_ATTR_RO(committable_child_, committed);
+
+static struct configfs_attribute *committable_child_attrs[] = {
+ _child_attr_storeme,
+ _child_attr_committed,
+ NULL,
+};
+
+static void committable_child_release(struct config_item *item)
+{
+ kfree(to_committable_child(item));
+}
+
+static struct configfs_item_operations committable_child_item_ops = {
+ .release= committable_child_release,
+};
+
+static const struct config_item_type committable_child_type = {
+ .ct_item_ops= _child_item_ops,
+ .ct_attrs = committable_child_attrs,
+ .ct_owner = THIS_MODULE,
+};
+
+struct committable_children {
+ struct config_group group;
+};
+
+static struct config_item *
+committable_children_make_item(struct config_group *group, const char *name)
+{
+ struct committable_child *child;
+
+ child = kzalloc(sizeof(*child), GFP_KERNEL);
+ if (!child)
+ return ERR_PTR(-ENOMEM);
+
+ config_item_init_type_name(>item, name, _child_type);
+
+ return >item;
+}
+
+static ssize_t
+committable_children_description_show(struct config_item *item, char *page)
+{
+ return sprintf(page,
+"[04-committable-children]\n"
+"\n"
+"This subsystem allows creation of committable config_items. The subsystem\n"
+"has two subdirectories: pending and live. New config_items can only be\n"
+"created in pending/ and they have one writable and readable attribute as\n"
+"well as a single read-only attribute. The latter is only changed once the\n"
+"item is 'committed'. This is done by moving the config_item (using\n"
+"rename()) to the live/ directory. In this example, the storeme attribute\n"
+"becomes 'read-only' once committed.\n");
+}
+
+CONFIGFS_ATTR_RO(committable_children_, description);
+
+static struct configfs_attribute *committable_children_attrs[] = {
+ _children_attr_description,
+ NULL,
+};
+
+static int committable_children_commit_item(struct config_item *item)
+{
+ to_committable_child(item)->committed = true;
+
+ return 0;
+}
+
+static int committable_children_uncommit_item(struct config_item *item)
+{
+ to_committable_child(item)->committed = false;
+
+ return 0;
+}
+
+static struct configfs_group_operations committable_children_group_ops = {
+ .make_item = committable_children_make_item,
+ .commit_item= committable_children_commit_item,
+ .uncommit_item = committable_children
[PATCH v5 03/11] configfs: implement committable items
From: Bartosz Golaszewski
This implements configfs committable items. We mostly follow the
documentation except that we extend config_group_ops with uncommit_item()
callback for reverting the changes made by commit_item().
Each committable group has two sub-directories: pending and live. New
items can only be created in pending/. Attributes can only be modified
while the item is in pending/. Once it's ready to be committed, it must
be moved over to live/ using the rename() system call. This is when the
commit_item() function will be called.
Implementation-wise: we reuse the default group mechanism to elegantly
plug the new pseude-groups into configfs. The pending group inherits the
parent group's operations so that config_items can be seamlesly created
in it using the callbacks supplied by the user as part of the committable
group itself.
Signed-off-by: Bartosz Golaszewski
Acked-by: Linus Walleij
---
Documentation/filesystems/configfs.rst | 6 +-
fs/configfs/configfs_internal.h| 2 +
fs/configfs/dir.c | 245 -
include/linux/configfs.h | 1 +
4 files changed, 245 insertions(+), 9 deletions(-)
diff --git a/Documentation/filesystems/configfs.rst
b/Documentation/filesystems/configfs.rst
index 1d3d6f4a82a9..7e0e7c356450 100644
--- a/Documentation/filesystems/configfs.rst
+++ b/Documentation/filesystems/configfs.rst
@@ -290,6 +290,7 @@ config_item_type::
struct config_group *(*make_group)(struct config_group *group,
const char *name);
int (*commit_item)(struct config_item *item);
+ int (*uncommit_item)(struct config_item *item);
void (*disconnect_notify)(struct config_group *group,
struct config_item *item);
void (*drop_item)(struct config_group *group,
@@ -490,9 +491,6 @@ pass up an error.
Committable Items
=
-Note:
- Committable items are currently unimplemented.
-
Some config_items cannot have a valid initial state. That is, no
default values can be specified for the item's attributes such that the
item can do its work. Userspace must configure one or more attributes,
@@ -532,4 +530,4 @@ method returns zero and the item is moved to the "live"
directory.
As rmdir(2) does not work in the "live" directory, an item must be
shutdown, or "uncommitted". Again, this is done via rename(2), this
time from the "live" directory back to the "pending" one. The subsystem
-is notified by the ct_group_ops->uncommit_object() method.
+is notified by the ct_group_ops->uncommit_item() method.
diff --git a/fs/configfs/configfs_internal.h b/fs/configfs/configfs_internal.h
index b495c9f043d4..41ac21c82bf5 100644
--- a/fs/configfs/configfs_internal.h
+++ b/fs/configfs/configfs_internal.h
@@ -56,6 +56,8 @@ struct configfs_dirent {
#define CONFIGFS_USET_DROPPING (1UL << 8)
#define CONFIGFS_USET_IN_MKDIR (1UL << 9)
#define CONFIGFS_USET_CREATING (1UL << 10)
+#define CONFIGFS_GROUP_PENDING (1UL << 11)
+#define CONFIGFS_GROUP_LIVE(1UL << 12)
#define CONFIGFS_NOT_PINNED(CONFIGFS_ITEM_ATTR | CONFIGFS_ITEM_BIN_ATTR)
extern struct mutex configfs_symlink_mutex;
diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
index b6098e02e20b..f3c95c1d5278 100644
--- a/fs/configfs/dir.c
+++ b/fs/configfs/dir.c
@@ -656,6 +656,13 @@ static void detach_groups(struct config_group *group)
inode_unlock(d_inode(child));
+ /*
+* Free memory allocated for the pending and live directories
+* of committable groups.
+*/
+ if (sd->s_type & (CONFIGFS_GROUP_PENDING | CONFIGFS_GROUP_LIVE))
+ kfree(sd->s_element);
+
d_delete(child);
dput(child);
}
@@ -860,6 +867,134 @@ static void configfs_detach_item(struct config_item *item)
configfs_remove_dir(item);
}
+static bool is_committable_group(struct config_item *item)
+{
+ const struct config_item_type *type = item->ci_type;
+
+ if (type && type->ct_group_ops &&
+ type->ct_group_ops->commit_item &&
+ type->ct_group_ops->uncommit_item)
+ return true;
+
+ return false;
+}
+
+struct pending_group_data {
+ struct config_group group;
+ struct config_item_type type;
+ struct configfs_group_operations group_ops;
+};
+
+struct live_group_data {
+ struct config_group group;
+ struct config_item_type type;
+};
+
+static int create_pending_group(struct config_item *parent_item,
+ struct configfs_fragment *frag)
+{
+ const struct config_item_type *parent_type = parent_item->
Re: [PATCH v1 1/1] gpiolib: Read "gpio-line-names" from a firmware node
On Fri, Mar 5, 2021 at 1:03 PM Andy Shevchenko
wrote:
>
> On STM32MP1, the GPIO banks are subnodes of pin-controller@50002000,
> see arch/arm/boot/dts/stm32mp151.dtsi. The driver for
> pin-controller@50002000 is in drivers/pinctrl/stm32/pinctrl-stm32.c
> and iterates over all of its DT subnodes when registering each GPIO
> bank gpiochip. Each gpiochip has:
>
> - gpio_chip.parent = dev,
> where dev is the device node of the pin controller
> - gpio_chip.of_node = np,
> which is the OF node of the GPIO bank
>
> Therefore, dev_fwnode(chip->parent) != of_fwnode_handle(chip.of_node),
> i.e. pin-controller@50002000 != pin-controller@50002000/gpio@5000*000.
>
> The original code behaved correctly, as it extracted the "gpio-line-names"
> from of_fwnode_handle(chip.of_node) = pin-controller@50002000/gpio@5000*000.
>
> To achieve the same behaviour, read property from the firmware node.
>
> Fixes: 7cba1a4d5e162 ("gpiolib: generalize devprop_gpiochip_set_names() for
> device properties")
> Reported-by: Marek Vasut
> Reported-by: Roman Guskov
> Signed-off-by: Andy Shevchenko
> ---
> drivers/gpio/gpiolib.c | 12
> 1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 3bc25a9c4cd6..ba88011cc79d 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -367,22 +367,18 @@ static int gpiochip_set_desc_names(struct gpio_chip *gc)
> *
> * Looks for device property "gpio-line-names" and if it exists assigns
> * GPIO line names for the chip. The memory allocated for the assigned
> - * names belong to the underlying software node and should not be released
> + * names belong to the underlying firmware node and should not be released
> * by the caller.
> */
> static int devprop_gpiochip_set_names(struct gpio_chip *chip)
> {
> struct gpio_device *gdev = chip->gpiodev;
> - struct device *dev = chip->parent;
> + struct fwnode_handle *fwnode = dev_fwnode(>dev);
> const char **names;
> int ret, i;
> int count;
>
> - /* GPIO chip may not have a parent device whose properties we
> inspect. */
> - if (!dev)
> - return 0;
> -
> - count = device_property_string_array_count(dev, "gpio-line-names");
> + count = fwnode_property_string_array_count(fwnode, "gpio-line-names");
> if (count < 0)
> return 0;
>
> @@ -396,7 +392,7 @@ static int devprop_gpiochip_set_names(struct gpio_chip
> *chip)
> if (!names)
> return -ENOMEM;
>
> - ret = device_property_read_string_array(dev, "gpio-line-names",
> + ret = fwnode_property_read_string_array(fwnode, "gpio-line-names",
> names, count);
> if (ret < 0) {
> dev_warn(>dev, "failed to read GPIO line names\n");
> --
> 2.30.1
>
Hi Andy!
Unfortunately while this may fix the particular use-case on STM32, it
breaks all other users as the 'gpio-line-names' property doesn't live
on dev_fwnode(>dev) but on dev_fwnode(chip->parent).
How about we first look for this property on the latter and only if
it's not present descend down to the former fwnode?
Bart
Re: [PATCH v6 0/6] gpiolib: switch to fwnode in the core
On Tue, Mar 9, 2021 at 10:37 AM Andy Shevchenko wrote: > > GPIO library uses of_node and fwnode in the core in non-unified way. > The series cleans this up and improves IRQ domain creation for non-OF cases > where currently the names of the domain are 'unknown'. > > This has been tested on Intel Galileo Gen 2. > > It touches GPIO core parts and it's expected that the series is routed via > GPIO tree. > > In v6: > - added tag to the patch 5 (Rafael) > - dropped ops temporary variable (Rafael) > > In v5: > - same as v4 + v3 (patches 1-4/5) in order to route via GPIO tree (Bart) > > In v4: > - based on Rafael's bleeding-edge > - split the rest to two patches (Rafael) > - elaborate WARN() deduplication in the commit message (Rafael) > > In v3: > - fixed subtle bug in gpiod_count > - made irq_domain_add_simple() static inline (Marc) > > In v2: > - added a new patch due to functionality in irq_comain_add_simple() (Linus) > - tagged patches 2-4 (Linus) > - Cc'ed to Rafael > > Andy Shevchenko (6): > irqdomain: Introduce irq_domain_create_simple() API > gpiolib: Unify the checks on fwnode type > gpiolib: Move of_node operations to gpiolib-of and correct fwnode use > gpiolib: Introduce acpi_gpio_dev_init() and call it from core > gpiolib: Reuse device's fwnode to create IRQ domain > gpiolib: Fold conditionals into a simple ternary operator > > Documentation/core-api/irq/irq-domain.rst | 22 > drivers/gpio/gpiolib-acpi.c | 7 +++ > drivers/gpio/gpiolib-acpi.h | 4 ++ > drivers/gpio/gpiolib-of.c | 6 ++- > drivers/gpio/gpiolib.c| 62 +-- > include/linux/irqdomain.h | 19 +-- > kernel/irq/irqdomain.c| 20 > 7 files changed, 75 insertions(+), 65 deletions(-) > > -- > 2.30.1 > Series applied, thanks! Bartosz
Re: [PATCH v4 07/11] lib: bitmap: provide devm_bitmap_alloc() and devm_bitmap_zalloc()
On Fri, Mar 12, 2021 at 12:40 PM Andy Shevchenko wrote: > > On Fri, Mar 12, 2021 at 10:56:56AM +0100, Bartosz Golaszewski wrote: > > From: Bartosz Golaszewski > > > > Provide managed variants of bitmap_alloc() and bitmap_zalloc(). > > Perhaps I missed your answer to Greg's comment... > So, what do you think about adding _GPL to the export? > > -- > With Best Regards, > Andy Shevchenko > > Nah, I missed Greg's comment. :( Will fix in v5. Bart
Re: [PATCH] gpio: mpc8xxx: Add ACPI support
On Fri, Mar 12, 2021 at 7:51 AM Ran Wang wrote:
>
> Current implementation only supports DT, now add ACPI support.
>
> Note that compared to device of 'fsl,qoriq-gpio', LS1028A and
> LS1088A's GPIO have no extra programming, so simplify related checking.
>
> Signed-off-by: Ran Wang
> ---
> drivers/gpio/gpio-mpc8xxx.c | 50 +++--
> 1 file changed, 37 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c
> index 6dfca83bcd90..de5b7e17cde3 100644
> --- a/drivers/gpio/gpio-mpc8xxx.c
> +++ b/drivers/gpio/gpio-mpc8xxx.c
> @@ -9,6 +9,7 @@
> * kind, whether express or implied.
> */
>
> +#include
> #include
> #include
> #include
> @@ -292,8 +293,6 @@ static const struct of_device_id mpc8xxx_gpio_ids[] = {
> { .compatible = "fsl,mpc5121-gpio", .data = _gpio_devtype, },
> { .compatible = "fsl,mpc5125-gpio", .data = _gpio_devtype, },
> { .compatible = "fsl,pq3-gpio", },
> - { .compatible = "fsl,ls1028a-gpio", },
> - { .compatible = "fsl,ls1088a-gpio", },
Why are you removing support for those?
Bart
> { .compatible = "fsl,qoriq-gpio", },
> {}
> };
> @@ -303,10 +302,19 @@ static int mpc8xxx_probe(struct platform_device *pdev)
> struct device_node *np = pdev->dev.of_node;
> struct mpc8xxx_gpio_chip *mpc8xxx_gc;
> struct gpio_chip*gc;
> - const struct mpc8xxx_gpio_devtype *devtype =
> - of_device_get_match_data(>dev);
> + const struct mpc8xxx_gpio_devtype *devtype;
> + const struct acpi_device_id *acpi_id;
> int ret;
>
> + if (pdev->dev.of_node) {
> + devtype = of_device_get_match_data(>dev);
> + } else {
> + acpi_id =
> acpi_match_device(pdev->dev.driver->acpi_match_table,
> + >dev);
> + if (acpi_id)
> + devtype = (struct mpc8xxx_gpio_devtype *)
> acpi_id->driver_data;
> + }
> +
> mpc8xxx_gc = devm_kzalloc(>dev, sizeof(*mpc8xxx_gc),
> GFP_KERNEL);
> if (!mpc8xxx_gc)
> return -ENOMEM;
> @@ -315,14 +323,14 @@ static int mpc8xxx_probe(struct platform_device *pdev)
>
> raw_spin_lock_init(_gc->lock);
>
> - mpc8xxx_gc->regs = of_iomap(np, 0);
> + mpc8xxx_gc->regs = devm_platform_ioremap_resource(pdev, 0);
> if (!mpc8xxx_gc->regs)
> return -ENOMEM;
>
> gc = _gc->gc;
> gc->parent = >dev;
>
> - if (of_property_read_bool(np, "little-endian")) {
> + if (device_property_read_bool(>dev, "little-endian")) {
> ret = bgpio_init(gc, >dev, 4,
> mpc8xxx_gc->regs + GPIO_DAT,
> NULL, NULL,
> @@ -369,10 +377,14 @@ static int mpc8xxx_probe(struct platform_device *pdev)
> * associated input enable must be set (GPIOxGPIE[IEn]=1) to propagate
> * the port value to the GPIO Data Register.
> */
> - if (of_device_is_compatible(np, "fsl,qoriq-gpio") ||
> - of_device_is_compatible(np, "fsl,ls1028a-gpio") ||
> - of_device_is_compatible(np, "fsl,ls1088a-gpio"))
> - gc->write_reg(mpc8xxx_gc->regs + GPIO_IBE, 0x);
> + if (pdev->dev.of_node) {
> + if (of_device_is_compatible(np, "fsl,qoriq-gpio"))
> + gc->write_reg(mpc8xxx_gc->regs + GPIO_IBE,
> 0x);
> + } else {
> + if (acpi_match_device(pdev->dev.driver->acpi_match_table,
> + >dev))
> + gc->write_reg(mpc8xxx_gc->regs + GPIO_IBE,
> 0x);
> + }
>
> ret = gpiochip_add_data(gc, mpc8xxx_gc);
> if (ret) {
> @@ -381,12 +393,15 @@ static int mpc8xxx_probe(struct platform_device *pdev)
> goto err;
> }
>
> - mpc8xxx_gc->irqn = irq_of_parse_and_map(np, 0);
> + mpc8xxx_gc->irqn = platform_get_irq(pdev, 0);
> if (!mpc8xxx_gc->irqn)
> return 0;
>
> - mpc8xxx_gc->irq = irq_domain_add_linear(np, MPC8XXX_GPIO_PINS,
> - _gpio_irq_ops, mpc8xxx_gc);
> + mpc8xxx_gc->irq = irq_domain_create_linear(dev_fwnode(>dev),
> + MPC8XXX_GPIO_PINS,
> + _gpio_irq_ops,
> + mpc8xxx_gc);
> +
> if (!mpc8xxx_gc->irq)
> return 0;
>
> @@ -425,12 +440,21 @@ static int mpc8xxx_remove(struct platform_device *pdev)
> return 0;
> }
>
> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id gpio_acpi_ids[] = {
> + {"NXP0031",},
> + { }
> +};
> +MODULE_DEVICE_TABLE(acpi, gpio_acpi_ids);
> +#endif
> +
> static struct platform_driver mpc8xxx_plat_driver = {
> .probe
[PATCH v4 11/11] selftests: gpio: add test cases for gpio-sim
From: Bartosz Golaszewski
Add a set of tests for the new gpio-sim module. This is a pure shell
test-suite and uses the helper programs available in the gpio selftests
directory. These test-cases only test the functionalities exposed by the
gpio-sim driver, not those handled by core gpiolib code.
Signed-off-by: Bartosz Golaszewski
---
tools/testing/selftests/gpio/Makefile| 2 +-
tools/testing/selftests/gpio/config | 1 +
tools/testing/selftests/gpio/gpio-sim.sh | 229 +++
3 files changed, 231 insertions(+), 1 deletion(-)
create mode 100755 tools/testing/selftests/gpio/gpio-sim.sh
diff --git a/tools/testing/selftests/gpio/Makefile
b/tools/testing/selftests/gpio/Makefile
index d7d8f1985d99..4c6df61c76a8 100644
--- a/tools/testing/selftests/gpio/Makefile
+++ b/tools/testing/selftests/gpio/Makefile
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
-TEST_PROGS := gpio-mockup.sh
+TEST_PROGS := gpio-mockup.sh gpio-sim.sh
TEST_FILES := gpio-mockup-sysfs.sh
TEST_GEN_PROGS_EXTENDED := gpio-mockup-cdev gpio-chip-info gpio-line-name
diff --git a/tools/testing/selftests/gpio/config
b/tools/testing/selftests/gpio/config
index ce100342c20b..409a8532facc 100644
--- a/tools/testing/selftests/gpio/config
+++ b/tools/testing/selftests/gpio/config
@@ -1,3 +1,4 @@
CONFIG_GPIOLIB=y
CONFIG_GPIO_CDEV=y
CONFIG_GPIO_MOCKUP=m
+CONFIG_GPIO_SIM=m
diff --git a/tools/testing/selftests/gpio/gpio-sim.sh
b/tools/testing/selftests/gpio/gpio-sim.sh
new file mode 100755
index ..fcca6ec611f8
--- /dev/null
+++ b/tools/testing/selftests/gpio/gpio-sim.sh
@@ -0,0 +1,229 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2021 Bartosz Golaszewski
+
+BASE_DIR=`dirname $0`
+CONFIGFS_DIR="/sys/kernel/config/gpio-sim"
+PENDING_DIR=$CONFIGFS_DIR/pending
+LIVE_DIR=$CONFIGFS_DIR/live
+MODULE="gpio-sim"
+
+fail() {
+ echo "$*" >&2
+ echo "GPIO $MODULE test FAIL"
+ exit 1
+}
+
+skip() {
+ echo "$*" >&2
+ echo "GPIO $MODULE test SKIP"
+ exit 4
+}
+
+configfs_cleanup() {
+ for DIR in `ls $LIVE_DIR`; do
+ mv $LIVE_DIR/$DIR $PENDING_DIR
+ done
+
+ for DIR in `ls $PENDING_DIR`; do
+ rmdir $PENDING_DIR/$DIR
+ done
+}
+
+create_pending_chip() {
+ local NAME="$1"
+ local LABEL="$2"
+ local NUM_LINES="$3"
+ local LINE_NAMES="$4"
+ local CHIP_DIR="$PENDING_DIR/$NAME"
+
+ mkdir $CHIP_DIR
+ test -n "$LABEL" && echo $LABEL > $CHIP_DIR/label
+ test -n "$NUM_LINES" && echo $NUM_LINES > $CHIP_DIR/num_lines
+ if [ -n "$LINE_NAMES" ]; then
+ echo $LINE_NAMES 2> /dev/null > $CHIP_DIR/line_names
+ # This one can fail
+ if [ "$?" -ne "0" ]; then
+ return 1
+ fi
+ fi
+}
+
+create_live_chip() {
+ local CHIP_DIR="$PENDING_DIR/$1"
+
+ create_pending_chip "$@" || fail "unable to create the chip configfs
item"
+ mv $CHIP_DIR $LIVE_DIR || fail "unable to commit the chip configfs item"
+}
+
+remove_pending_chip() {
+ local NAME="$1"
+
+ rmdir $PENDING_DIR/$NAME || fail "unable to remove the chip configfs
item"
+}
+
+remove_live_chip() {
+ local NAME="$1"
+
+ mv $LIVE_DIR/$NAME $PENDING_DIR || fail "unable to uncommit the chip
configfs item"
+ remove_pending_chip "$@"
+}
+
+configfs_chip_name() {
+ local CHIP="$1"
+
+ cat $LIVE_DIR/$CHIP/chip_name 2> /dev/null || return 1
+}
+
+configfs_dev_name() {
+ local CHIP="$1"
+
+ cat $LIVE_DIR/$CHIP/dev_name 2> /dev/null || return 1
+}
+
+get_chip_num_lines() {
+ local CHIP="$1"
+
+ $BASE_DIR/gpio-chip-info /dev/`configfs_chip_name $CHIP` num-lines
+}
+
+get_chip_label() {
+ local CHIP="$1"
+
+ $BASE_DIR/gpio-chip-info /dev/`configfs_chip_name $CHIP` label
+}
+
+get_line_name() {
+ local CHIP="$1"
+ local OFFSET="$2"
+
+ $BASE_DIR/gpio-line-name /dev/`configfs_chip_name $CHIP` $OFFSET
+}
+
+sysfs_set_pull() {
+ local CHIP="$1"
+ local OFFSET="$2"
+ local PULL="$3"
+ local SYSFSPATH="/sys/devices/platform/`configfs_dev_name
$CHIP`/line-ctrl/gpio$OFFSET"
+
+ echo $PULL > $SYSFSPATH
+}
+
+# Load the gpio-sim module. This will pull in configfs if needed too.
+modprobe gpio-sim || skip "unable to load the gpio-sim module"
+# Make sure configfs is mounted at /sys/kernel/config. Wait a bit if needed.
+for IDX in `seq 5`; do
+ if [ "$IDX" -eq "5" ]; then
+ skip
[PATCH v4 10/11] selftests: gpio: add a helper for reading GPIO line names
From: Bartosz Golaszewski
Add a simple program that allows to read GPIO line names from the
character device. This will be used in gpio-sim selftests.
Signed-off-by: Bartosz Golaszewski
---
tools/testing/selftests/gpio/.gitignore | 1 +
tools/testing/selftests/gpio/Makefile | 2 +-
tools/testing/selftests/gpio/gpio-line-name.c | 55 +++
3 files changed, 57 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/gpio/gpio-line-name.c
diff --git a/tools/testing/selftests/gpio/.gitignore
b/tools/testing/selftests/gpio/.gitignore
index 4ea4f58dab1a..ededb077a3a6 100644
--- a/tools/testing/selftests/gpio/.gitignore
+++ b/tools/testing/selftests/gpio/.gitignore
@@ -1,3 +1,4 @@
# SPDX-License-Identifier: GPL-2.0-only
gpio-mockup-cdev
gpio-chip-info
+gpio-line-name
diff --git a/tools/testing/selftests/gpio/Makefile
b/tools/testing/selftests/gpio/Makefile
index 84b48547f94c..d7d8f1985d99 100644
--- a/tools/testing/selftests/gpio/Makefile
+++ b/tools/testing/selftests/gpio/Makefile
@@ -2,6 +2,6 @@
TEST_PROGS := gpio-mockup.sh
TEST_FILES := gpio-mockup-sysfs.sh
-TEST_GEN_PROGS_EXTENDED := gpio-mockup-cdev gpio-chip-info
+TEST_GEN_PROGS_EXTENDED := gpio-mockup-cdev gpio-chip-info gpio-line-name
include ../lib.mk
diff --git a/tools/testing/selftests/gpio/gpio-line-name.c
b/tools/testing/selftests/gpio/gpio-line-name.c
new file mode 100644
index ..a52e75bc37ba
--- /dev/null
+++ b/tools/testing/selftests/gpio/gpio-line-name.c
@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * GPIO character device helper for reading line names.
+ *
+ * Copyright (C) 2021 Bartosz Golaszewski
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+static void print_usage(void)
+{
+ printf("usage:\n");
+ printf(" gpio-line-name \n");
+}
+
+int main(int argc, char **argv)
+{
+ struct gpio_v2_line_info info;
+ int fd, ret;
+ char *endp;
+
+ if (argc != 3) {
+ print_usage();
+ return EXIT_FAILURE;
+ }
+
+ fd = open(argv[1], O_RDWR);
+ if (fd < 0) {
+ perror("unable to open the GPIO chip");
+ return EXIT_FAILURE;
+ }
+
+ memset(, 0, sizeof(info));
+ info.offset = strtoul(argv[2], , 10);
+ if (*endp != '\0') {
+ print_usage();
+ return EXIT_FAILURE;
+ }
+
+ ret = ioctl(fd, GPIO_V2_GET_LINEINFO_IOCTL, );
+ if (ret) {
+ perror("line info ioctl failed");
+ return EXIT_FAILURE;
+ }
+
+ printf("%s\n", info.name);
+
+ return EXIT_SUCCESS;
+}
--
2.30.1
[PATCH v4 09/11] selftests: gpio: provide a helper for reading chip info
From: Bartosz Golaszewski
Add a simple program that allows to retrieve chip properties from the
GPIO character device. This will be used in gpio-sim selftests.
Signed-off-by: Bartosz Golaszewski
---
tools/testing/selftests/gpio/.gitignore | 1 +
tools/testing/selftests/gpio/Makefile | 2 +-
tools/testing/selftests/gpio/gpio-chip-info.c | 57 +++
3 files changed, 59 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/gpio/gpio-chip-info.c
diff --git a/tools/testing/selftests/gpio/.gitignore
b/tools/testing/selftests/gpio/.gitignore
index a4969f7ee020..4ea4f58dab1a 100644
--- a/tools/testing/selftests/gpio/.gitignore
+++ b/tools/testing/selftests/gpio/.gitignore
@@ -1,2 +1,3 @@
# SPDX-License-Identifier: GPL-2.0-only
gpio-mockup-cdev
+gpio-chip-info
diff --git a/tools/testing/selftests/gpio/Makefile
b/tools/testing/selftests/gpio/Makefile
index 39f2bbe8dd3d..84b48547f94c 100644
--- a/tools/testing/selftests/gpio/Makefile
+++ b/tools/testing/selftests/gpio/Makefile
@@ -2,6 +2,6 @@
TEST_PROGS := gpio-mockup.sh
TEST_FILES := gpio-mockup-sysfs.sh
-TEST_GEN_PROGS_EXTENDED := gpio-mockup-cdev
+TEST_GEN_PROGS_EXTENDED := gpio-mockup-cdev gpio-chip-info
include ../lib.mk
diff --git a/tools/testing/selftests/gpio/gpio-chip-info.c
b/tools/testing/selftests/gpio/gpio-chip-info.c
new file mode 100644
index ..4d26fa7c254a
--- /dev/null
+++ b/tools/testing/selftests/gpio/gpio-chip-info.c
@@ -0,0 +1,57 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * GPIO character device helper for reading chip information.
+ *
+ * Copyright (C) 2021 Bartosz Golaszewski
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+static void print_usage(void)
+{
+ printf("usage:\n");
+ printf(" gpio-chip-info [name|label|num-lines]\n");
+}
+
+int main(int argc, char **argv)
+{
+ struct gpiochip_info info;
+ int fd, ret;
+
+ if (argc !=3) {
+ print_usage();
+ return EXIT_FAILURE;
+ }
+
+ fd = open(argv[1], O_RDWR);
+ if (fd < 0) {
+ perror("unable to open the GPIO chip");
+ return EXIT_FAILURE;
+ }
+
+ memset(, 0, sizeof(info));
+ ret = ioctl(fd, GPIO_GET_CHIPINFO_IOCTL, );
+ if (ret) {
+ perror("chip info ioctl failed");
+ return EXIT_FAILURE;
+ }
+
+ if (strcmp(argv[2], "name") == 0) {
+ printf("%s\n", info.name);
+ } else if (strcmp(argv[2], "label") == 0) {
+ printf("%s\n", info.label);
+ } else if (strcmp(argv[2], "num-lines") == 0) {
+ printf("%u\n", info.lines);
+ } else {
+ fprintf(stderr, "unknown command: %s\n", argv[2]);
+ return EXIT_FAILURE;
+ }
+
+ return EXIT_SUCCESS;
+}
--
2.30.1
[PATCH v4 06/11] lib: bitmap: order includes alphabetically
From: Bartosz Golaszewski For better readability and maintenance: order the includes in bitmap source files alphabetically. Signed-off-by: Bartosz Golaszewski Reviewed-by: Andy Shevchenko --- include/linux/bitmap.h | 4 ++-- lib/bitmap.c | 9 + 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h index 6939a8983026..3282db97e06c 100644 --- a/include/linux/bitmap.h +++ b/include/linux/bitmap.h @@ -4,10 +4,10 @@ #ifndef __ASSEMBLY__ -#include #include -#include #include +#include +#include /* * bitmaps provide bit arrays that consume one or more unsigned diff --git a/lib/bitmap.c b/lib/bitmap.c index 75006c4036e9..78f70d9007ad 100644 --- a/lib/bitmap.c +++ b/lib/bitmap.c @@ -3,17 +3,18 @@ * lib/bitmap.c * Helper functions for bitmap.h. */ -#include -#include -#include -#include + #include #include #include +#include +#include +#include #include #include #include #include +#include #include #include -- 2.30.1
[PATCH v4 08/11] gpio: sim: new testing module
From: Bartosz Golaszewski Implement a new, modern GPIO testing module controlled by configfs attributes instead of module parameters. The goal of this driver is to provide a replacement for gpio-mockup that will be easily extensible with new features and doesn't require reloading the module to change the setup. Signed-off-by: Bartosz Golaszewski Reviewed-by: Andy Shevchenko Reviewed-by: Linus Walleij --- Documentation/admin-guide/gpio/gpio-sim.rst | 72 ++ drivers/gpio/Kconfig| 8 + drivers/gpio/Makefile | 1 + drivers/gpio/gpio-sim.c | 874 4 files changed, 955 insertions(+) create mode 100644 Documentation/admin-guide/gpio/gpio-sim.rst create mode 100644 drivers/gpio/gpio-sim.c diff --git a/Documentation/admin-guide/gpio/gpio-sim.rst b/Documentation/admin-guide/gpio/gpio-sim.rst new file mode 100644 index ..08eac487e35e --- /dev/null +++ b/Documentation/admin-guide/gpio/gpio-sim.rst @@ -0,0 +1,72 @@ +.. SPDX-License-Identifier: GPL-2.0-or-later + +Configfs GPIO Simulator +=== + +The configfs GPIO Simulator (gpio-sim) provides a way to create simulated GPIO +chips for testing purposes. The lines exposed by these chips can be accessed +using the standard GPIO character device interface as well as manipulated +using sysfs attributes. + +Creating simulated chips + + +The gpio-sim module registers a configfs subsystem called 'gpio-sim'. It's a +subsystem with committable items which means two subdirectories are created in +the filesystem: pending and live. For more information on configfs and +committable items, please refer to Documentation/filesystems/configfs.rst. + +In order to instantiate a new simulated chip, the user needs to mkdir() a new +directory in pending/. Inside each new directory, there's a set of attributes +that can be used to configure the new chip. Once the configuration is complete, +the user needs to use rename() to move the chip to the live/ directory. This +creates and registers the new device. + +In order to destroy a simulated chip, it has to be moved back to pending first +and then removed using rmdir(). + +Currently supported configuration attributes are: + + num_lines - an unsigned integer value defining the number of GPIO lines to + export + + label - a string defining the label for the GPIO chip + + line_names - a list of GPIO line names in the form of quoted strings + separated by commas, e.g.: '"foo", "bar", "", "foobar"'. The + number of strings doesn't have to be equal to the value set in + the num_lines attribute. If it's lower than the number of lines, + the remaining lines are unnamed. If it's larger, the superfluous + lines are ignored. A name of the form: '""' means the line + should be unnamed. + +Additionally two read-only attributes named 'chip_name' and 'dev_name' are +exposed in order to provide users with a mapping from configfs directories to +the actual devices created in the kernel. The former returns the name of the +GPIO device as assigned by gpiolib (i.e. "gpiochip0", "gpiochip1", etc.). The +latter returns the parent device name as defined by the gpio-sim driver (i.e. +"gpio-sim.0", "gpio-sim.1", etc.). This allows user-space to map the configfs +items both to the correct character device file as well as the associated entry +in sysfs. + +Simulated GPIO chips can also be defined in device-tree. The compatible string +must be: "gpio-simulator". Supported properties are: + + "gpio-sim,label" - chip label + + "gpio-sim,nr-gpios" - number of lines + +Other standard GPIO properties (like "gpio-line-names" and gpio-hog) are also +supported. + +Manipulating simulated lines + + +Each simulated GPIO chip creates a sysfs attribute group under its device +directory called 'line-ctrl'. Inside each group, there's a separate attribute +for each GPIO line. The name of the attribute is of the form 'gpioX' where X +is the line's offset in the chip. + +Reading from a line attribute returns the current value. Writing to it (0 or 1) +changes the configuration of the simulated pull-up/pull-down resistor +(1 - pull-up, 0 - pull-down). diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index e3607ec4c2e8..b6b6150d0d04 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -1633,6 +1633,14 @@ config GPIO_MOCKUP tools/testing/selftests/gpio/gpio-mockup.sh. Reference the usage in it. +config GPIO_SIM + tristate "GPIO Simulator Module" + select IRQ_SIM + select CONFIGFS_FS + help + This enables the GPIO simulator - a configfs-based GPIO testing + driver. + endmenu endif diff
[PATCH v4 07/11] lib: bitmap: provide devm_bitmap_alloc() and devm_bitmap_zalloc()
From: Bartosz Golaszewski
Provide managed variants of bitmap_alloc() and bitmap_zalloc().
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Andy Shevchenko
---
include/linux/bitmap.h | 8
lib/bitmap.c | 33 +
2 files changed, 41 insertions(+)
diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index 3282db97e06c..73d039476fa4 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -9,6 +9,8 @@
#include
#include
+struct device;
+
/*
* bitmaps provide bit arrays that consume one or more unsigned
* longs. The bitmap interface and available operations are listed
@@ -122,6 +124,12 @@ unsigned long *bitmap_alloc(unsigned int nbits, gfp_t
flags);
unsigned long *bitmap_zalloc(unsigned int nbits, gfp_t flags);
void bitmap_free(const unsigned long *bitmap);
+/* Managed variants of the above. */
+unsigned long *devm_bitmap_alloc(struct device *dev,
+unsigned int nbits, gfp_t flags);
+unsigned long *devm_bitmap_zalloc(struct device *dev,
+ unsigned int nbits, gfp_t flags);
+
/*
* lib/bitmap.c provides these functions:
*/
diff --git a/lib/bitmap.c b/lib/bitmap.c
index 78f70d9007ad..b4fd7fd084c6 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -8,6 +8,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -1263,6 +1264,38 @@ void bitmap_free(const unsigned long *bitmap)
}
EXPORT_SYMBOL(bitmap_free);
+static void devm_bitmap_free(void *data)
+{
+ unsigned long *bitmap = data;
+
+ bitmap_free(bitmap);
+}
+
+unsigned long *devm_bitmap_alloc(struct device *dev,
+unsigned int nbits, gfp_t flags)
+{
+ unsigned long *bitmap;
+ int ret;
+
+ bitmap = bitmap_alloc(nbits, flags);
+ if (!bitmap)
+ return NULL;
+
+ ret = devm_add_action_or_reset(dev, devm_bitmap_free, bitmap);
+ if (ret)
+ return NULL;
+
+ return bitmap;
+}
+EXPORT_SYMBOL(devm_bitmap_alloc);
+
+unsigned long *devm_bitmap_zalloc(struct device *dev,
+ unsigned int nbits, gfp_t flags)
+{
+ return devm_bitmap_alloc(dev, nbits, flags | __GFP_ZERO);
+}
+EXPORT_SYMBOL(devm_bitmap_zalloc);
+
#if BITS_PER_LONG == 64
/**
* bitmap_from_arr32 - copy the contents of u32 array of bits to bitmap
--
2.30.1
[PATCH v4 04/11] samples: configfs: add a committable group
From: Bartosz Golaszewski
Add an example of using committable items to configfs samples. Each
config item has two attributes: read-write 'storeme' which works
similarly to other examples in this file and a read-only 'committed'
attribute which changes its value between false and true depending on
whether it's committed or not at the moment.
Signed-off-by: Bartosz Golaszewski
Acked-by: Linus Walleij
---
samples/configfs/configfs_sample.c | 153 +
1 file changed, 153 insertions(+)
diff --git a/samples/configfs/configfs_sample.c
b/samples/configfs/configfs_sample.c
index f9008be7a8a1..9bef74e4369d 100644
--- a/samples/configfs/configfs_sample.c
+++ b/samples/configfs/configfs_sample.c
@@ -315,6 +315,158 @@ static struct configfs_subsystem group_children_subsys = {
/* - */
+/*
+ * 04-committable-children
+ *
+ * This is an example of a committable group. It's similar to the simple
+ * children example but each config_item has an additional 'committed'
+ * attribute which is read-only and is only modified when the config_item
+ * is moved from the 'pending' to the 'live' directory.
+ */
+
+struct committable_child {
+ struct config_item item;
+ int storeme;
+ bool committed;
+};
+
+static inline struct committable_child *
+to_committable_child(struct config_item *item)
+{
+ return container_of(item, struct committable_child, item);
+}
+
+static ssize_t
+committable_child_storeme_show(struct config_item *item, char *page)
+{
+ return sprintf(page, "%d\n", to_committable_child(item)->storeme);
+}
+
+static ssize_t committable_child_storeme_store(struct config_item *item,
+ const char *page, size_t count)
+{
+ struct committable_child *child = to_committable_child(item);
+ int ret;
+
+ if (child->committed)
+ return -EPERM;
+
+ ret = kstrtoint(page, 10, >storeme);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+CONFIGFS_ATTR(committable_child_, storeme);
+
+static ssize_t
+committable_child_committed_show(struct config_item *item, char *page)
+{
+ return sprintf(page, "%s\n",
+ to_committable_child(item)->committed ? "true" : "false");
+}
+
+CONFIGFS_ATTR_RO(committable_child_, committed);
+
+static struct configfs_attribute *committable_child_attrs[] = {
+ _child_attr_storeme,
+ _child_attr_committed,
+ NULL,
+};
+
+static void committable_child_release(struct config_item *item)
+{
+ kfree(to_committable_child(item));
+}
+
+static struct configfs_item_operations committable_child_item_ops = {
+ .release= committable_child_release,
+};
+
+static const struct config_item_type committable_child_type = {
+ .ct_item_ops= _child_item_ops,
+ .ct_attrs = committable_child_attrs,
+ .ct_owner = THIS_MODULE,
+};
+
+struct committable_children {
+ struct config_group group;
+};
+
+static struct config_item *
+committable_children_make_item(struct config_group *group, const char *name)
+{
+ struct committable_child *child;
+
+ child = kzalloc(sizeof(*child), GFP_KERNEL);
+ if (!child)
+ return ERR_PTR(-ENOMEM);
+
+ config_item_init_type_name(>item, name, _child_type);
+
+ return >item;
+}
+
+static ssize_t
+committable_children_description_show(struct config_item *item, char *page)
+{
+ return sprintf(page,
+"[04-committable-children]\n"
+"\n"
+"This subsystem allows creation of committable config_items. The subsystem\n"
+"has two subdirectories: pending and live. New config_items can only be\n"
+"created in pending/ and they have one writable and readable attribute as\n"
+"well as a single read-only attribute. The latter is only changed once the\n"
+"item is 'committed'. This is done by moving the config_item (using\n"
+"rename()) to the live/ directory. In this example, the storeme attribute\n"
+"becomes 'read-only' once committed.\n");
+}
+
+CONFIGFS_ATTR_RO(committable_children_, description);
+
+static struct configfs_attribute *committable_children_attrs[] = {
+ _children_attr_description,
+ NULL,
+};
+
+static int committable_children_commit_item(struct config_item *item)
+{
+ to_committable_child(item)->committed = true;
+
+ return 0;
+}
+
+static int committable_children_uncommit_item(struct config_item *item)
+{
+ to_committable_child(item)->committed = false;
+
+ return 0;
+}
+
+static struct configfs_group_operations committable_children_group_ops = {
+ .make_item = committable_children_make_item,
+ .commit_item= committable_children_commit_item,
+ .uncommit_item = committable_children
[PATCH v4 03/11] configfs: implement committable items
From: Bartosz Golaszewski
This implements configfs committable items. We mostly follow the
documentation except that we extend config_group_ops with uncommit_item()
callback for reverting the changes made by commit_item().
Each committable group has two sub-directories: pending and live. New
items can only be created in pending/. Attributes can only be modified
while the item is in pending/. Once it's ready to be committed, it must
be moved over to live/ using the rename() system call. This is when the
commit_item() function will be called.
Implementation-wise: we reuse the default group mechanism to elegantly
plug the new pseude-groups into configfs. The pending group inherits the
parent group's operations so that config_items can be seamlesly created
in it using the callbacks supplied by the user as part of the committable
group itself.
Signed-off-by: Bartosz Golaszewski
Acked-by: Linus Walleij
---
Documentation/filesystems/configfs.rst | 6 +-
fs/configfs/configfs_internal.h| 2 +
fs/configfs/dir.c | 245 -
include/linux/configfs.h | 1 +
4 files changed, 245 insertions(+), 9 deletions(-)
diff --git a/Documentation/filesystems/configfs.rst
b/Documentation/filesystems/configfs.rst
index 1d3d6f4a82a9..7e0e7c356450 100644
--- a/Documentation/filesystems/configfs.rst
+++ b/Documentation/filesystems/configfs.rst
@@ -290,6 +290,7 @@ config_item_type::
struct config_group *(*make_group)(struct config_group *group,
const char *name);
int (*commit_item)(struct config_item *item);
+ int (*uncommit_item)(struct config_item *item);
void (*disconnect_notify)(struct config_group *group,
struct config_item *item);
void (*drop_item)(struct config_group *group,
@@ -490,9 +491,6 @@ pass up an error.
Committable Items
=
-Note:
- Committable items are currently unimplemented.
-
Some config_items cannot have a valid initial state. That is, no
default values can be specified for the item's attributes such that the
item can do its work. Userspace must configure one or more attributes,
@@ -532,4 +530,4 @@ method returns zero and the item is moved to the "live"
directory.
As rmdir(2) does not work in the "live" directory, an item must be
shutdown, or "uncommitted". Again, this is done via rename(2), this
time from the "live" directory back to the "pending" one. The subsystem
-is notified by the ct_group_ops->uncommit_object() method.
+is notified by the ct_group_ops->uncommit_item() method.
diff --git a/fs/configfs/configfs_internal.h b/fs/configfs/configfs_internal.h
index b495c9f043d4..41ac21c82bf5 100644
--- a/fs/configfs/configfs_internal.h
+++ b/fs/configfs/configfs_internal.h
@@ -56,6 +56,8 @@ struct configfs_dirent {
#define CONFIGFS_USET_DROPPING (1UL << 8)
#define CONFIGFS_USET_IN_MKDIR (1UL << 9)
#define CONFIGFS_USET_CREATING (1UL << 10)
+#define CONFIGFS_GROUP_PENDING (1UL << 11)
+#define CONFIGFS_GROUP_LIVE(1UL << 12)
#define CONFIGFS_NOT_PINNED(CONFIGFS_ITEM_ATTR | CONFIGFS_ITEM_BIN_ATTR)
extern struct mutex configfs_symlink_mutex;
diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
index b6098e02e20b..f3c95c1d5278 100644
--- a/fs/configfs/dir.c
+++ b/fs/configfs/dir.c
@@ -656,6 +656,13 @@ static void detach_groups(struct config_group *group)
inode_unlock(d_inode(child));
+ /*
+* Free memory allocated for the pending and live directories
+* of committable groups.
+*/
+ if (sd->s_type & (CONFIGFS_GROUP_PENDING | CONFIGFS_GROUP_LIVE))
+ kfree(sd->s_element);
+
d_delete(child);
dput(child);
}
@@ -860,6 +867,134 @@ static void configfs_detach_item(struct config_item *item)
configfs_remove_dir(item);
}
+static bool is_committable_group(struct config_item *item)
+{
+ const struct config_item_type *type = item->ci_type;
+
+ if (type && type->ct_group_ops &&
+ type->ct_group_ops->commit_item &&
+ type->ct_group_ops->uncommit_item)
+ return true;
+
+ return false;
+}
+
+struct pending_group_data {
+ struct config_group group;
+ struct config_item_type type;
+ struct configfs_group_operations group_ops;
+};
+
+struct live_group_data {
+ struct config_group group;
+ struct config_item_type type;
+};
+
+static int create_pending_group(struct config_item *parent_item,
+ struct configfs_fragment *frag)
+{
+ const struct config_item_type *parent_type = parent_item->
[PATCH v4 05/11] lib: bitmap: remove the 'extern' keyword from function declarations
From: Bartosz Golaszewski The 'extern' keyword doesn't have any benefits in header files. Remove it. Signed-off-by: Bartosz Golaszewski Reviewed-by: Andy Shevchenko --- include/linux/bitmap.h | 115 - 1 file changed, 57 insertions(+), 58 deletions(-) diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h index 70a932470b2d..6939a8983026 100644 --- a/include/linux/bitmap.h +++ b/include/linux/bitmap.h @@ -118,54 +118,53 @@ * Allocation and deallocation of bitmap. * Provided in lib/bitmap.c to avoid circular dependency. */ -extern unsigned long *bitmap_alloc(unsigned int nbits, gfp_t flags); -extern unsigned long *bitmap_zalloc(unsigned int nbits, gfp_t flags); -extern void bitmap_free(const unsigned long *bitmap); +unsigned long *bitmap_alloc(unsigned int nbits, gfp_t flags); +unsigned long *bitmap_zalloc(unsigned int nbits, gfp_t flags); +void bitmap_free(const unsigned long *bitmap); /* * lib/bitmap.c provides these functions: */ -extern int __bitmap_equal(const unsigned long *bitmap1, - const unsigned long *bitmap2, unsigned int nbits); -extern bool __pure __bitmap_or_equal(const unsigned long *src1, -const unsigned long *src2, -const unsigned long *src3, -unsigned int nbits); -extern void __bitmap_complement(unsigned long *dst, const unsigned long *src, - unsigned int nbits); -extern void __bitmap_shift_right(unsigned long *dst, const unsigned long *src, - unsigned int shift, unsigned int nbits); -extern void __bitmap_shift_left(unsigned long *dst, const unsigned long *src, - unsigned int shift, unsigned int nbits); -extern void bitmap_cut(unsigned long *dst, const unsigned long *src, - unsigned int first, unsigned int cut, - unsigned int nbits); -extern int __bitmap_and(unsigned long *dst, const unsigned long *bitmap1, +int __bitmap_equal(const unsigned long *bitmap1, + const unsigned long *bitmap2, unsigned int nbits); +bool __pure __bitmap_or_equal(const unsigned long *src1, + const unsigned long *src2, + const unsigned long *src3, + unsigned int nbits); +void __bitmap_complement(unsigned long *dst, const unsigned long *src, +unsigned int nbits); +void __bitmap_shift_right(unsigned long *dst, const unsigned long *src, + unsigned int shift, unsigned int nbits); +void __bitmap_shift_left(unsigned long *dst, const unsigned long *src, +unsigned int shift, unsigned int nbits); +void bitmap_cut(unsigned long *dst, const unsigned long *src, + unsigned int first, unsigned int cut, unsigned int nbits); +int __bitmap_and(unsigned long *dst, const unsigned long *bitmap1, +const unsigned long *bitmap2, unsigned int nbits); +void __bitmap_or(unsigned long *dst, const unsigned long *bitmap1, +const unsigned long *bitmap2, unsigned int nbits); +void __bitmap_xor(unsigned long *dst, const unsigned long *bitmap1, + const unsigned long *bitmap2, unsigned int nbits); +int __bitmap_andnot(unsigned long *dst, const unsigned long *bitmap1, + const unsigned long *bitmap2, unsigned int nbits); +void __bitmap_replace(unsigned long *dst, + const unsigned long *old, const unsigned long *new, + const unsigned long *mask, unsigned int nbits); +int __bitmap_intersects(const unsigned long *bitmap1, const unsigned long *bitmap2, unsigned int nbits); -extern void __bitmap_or(unsigned long *dst, const unsigned long *bitmap1, - const unsigned long *bitmap2, unsigned int nbits); -extern void __bitmap_xor(unsigned long *dst, const unsigned long *bitmap1, - const unsigned long *bitmap2, unsigned int nbits); -extern int __bitmap_andnot(unsigned long *dst, const unsigned long *bitmap1, - const unsigned long *bitmap2, unsigned int nbits); -extern void __bitmap_replace(unsigned long *dst, - const unsigned long *old, const unsigned long *new, - const unsigned long *mask, unsigned int nbits); -extern int __bitmap_intersects(const unsigned long *bitmap1, - const unsigned long *bitmap2, unsigned int nbits); -extern int __bitmap_subset(const unsigned long *bitmap1, - const unsigned long *bitmap2, unsigned int nbits); -extern int __bitmap_weight(const unsigned long *bitmap, unsigned int nbits); -extern void __bitmap_set(unsigned long *map, unsigned int start, int len); -extern void __bitmap_clear(unsigned long *map, unsigned int start, int len); - -extern unsigned
[PATCH v4 02/11] configfs: use (1UL << bit) for internal flags
From: Bartosz Golaszewski
For better readability and maintenance: use the (1UL << bit) for flag
definitions.
Signed-off-by: Bartosz Golaszewski
Acked-by: Linus Walleij
---
fs/configfs/configfs_internal.h | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/fs/configfs/configfs_internal.h b/fs/configfs/configfs_internal.h
index 9a3aed249692..b495c9f043d4 100644
--- a/fs/configfs/configfs_internal.h
+++ b/fs/configfs/configfs_internal.h
@@ -46,16 +46,16 @@ struct configfs_dirent {
struct configfs_fragment *s_frag;
};
-#define CONFIGFS_ROOT 0x0001
-#define CONFIGFS_DIR 0x0002
-#define CONFIGFS_ITEM_ATTR 0x0004
-#define CONFIGFS_ITEM_BIN_ATTR 0x0008
-#define CONFIGFS_ITEM_LINK 0x0020
-#define CONFIGFS_USET_DIR 0x0040
-#define CONFIGFS_USET_DEFAULT 0x0080
-#define CONFIGFS_USET_DROPPING 0x0100
-#define CONFIGFS_USET_IN_MKDIR 0x0200
-#define CONFIGFS_USET_CREATING 0x0400
+#define CONFIGFS_ROOT (1UL << 0)
+#define CONFIGFS_DIR (1UL << 1)
+#define CONFIGFS_ITEM_ATTR (1UL << 2)
+#define CONFIGFS_ITEM_BIN_ATTR (1UL << 3)
+#define CONFIGFS_ITEM_LINK (1UL << 5)
+#define CONFIGFS_USET_DIR (1UL << 6)
+#define CONFIGFS_USET_DEFAULT (1UL << 7)
+#define CONFIGFS_USET_DROPPING (1UL << 8)
+#define CONFIGFS_USET_IN_MKDIR (1UL << 9)
+#define CONFIGFS_USET_CREATING (1UL << 10)
#define CONFIGFS_NOT_PINNED(CONFIGFS_ITEM_ATTR | CONFIGFS_ITEM_BIN_ATTR)
extern struct mutex configfs_symlink_mutex;
--
2.30.1
[PATCH v4 01/11] configfs: increase the item name length
From: Bartosz Golaszewski 20 characters limit for item name is relatively small. Let's increase it to 32 to fit '04-committable-children' - a name we'll use in the sample code for committable items. Signed-off-by: Bartosz Golaszewski Acked-by: Linus Walleij --- include/linux/configfs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/configfs.h b/include/linux/configfs.h index 2e8c69b43c64..4f76dcc08134 100644 --- a/include/linux/configfs.h +++ b/include/linux/configfs.h @@ -27,7 +27,7 @@ #include/* struct kref */ #include /* struct mutex */ -#define CONFIGFS_ITEM_NAME_LEN 20 +#define CONFIGFS_ITEM_NAME_LEN 32 struct module; -- 2.30.1
[PATCH v4 00/11] gpio: implement the configfs testing module
From: Bartosz Golaszewski
This series adds a new GPIO testing module based on configfs committable items
and sysfs. The goal is to provide a testing driver that will be configurable
at runtime (won't need module reload) and easily extensible. The control over
the attributes is also much more fine-grained than in gpio-mockup.
This series also contains a respin of the patches I sent separately to the
configfs maintainers - these patches implement the concept of committable
items that was well defined for a long time but never actually completed.
Apart from the new driver itself, its selftests and the configfs patches, this
series contains some changes to the bitmap API - most importantly: it adds
devres managed variants of bitmap_alloc() and bitmap_zalloc().
v1 -> v2:
- add selftests for gpio-sim
- add helper programs for selftests
- update the configfs rename callback to work with the new API introduced in
v5.11
- fix a missing quote in the documentation
- use !! whenever using bits operation that are required to return 0 or 1
- use provided bitmap API instead of reimplementing copy or fill operations
- fix a deadlock in gpio_sim_direction_output()
- add new read-only configfs attributes for mapping of configfs items to GPIO
device names
- and address other minor issues pointed out in reviews of v1
v2 -> v3:
- use devm_bitmap_alloc() instead of the zalloc variant if we're initializing
the bitmap with 1s
- drop the patch exporting device_is_bound()
- don't return -ENODEV from dev_nam and chip_name configfs attributes, return
a string indicating that the device is not available yet ('n/a')
- fix indentation where it makes sense
- don't protect IDA functions which use their own locking and where it's not
needed
- use kmemdup() instead of kzalloc() + memcpy()
- collected review tags
- minor coding style fixes
v3 -> v4:
- return 'none' instead of 'n/a' from dev_name and chip_name before the device
is registered
- use sysfs_emit() instead of s*printf()
- drop GPIO_SIM_MAX_PROP as it's only used in an array's definition where it's
fine to hardcode the value
Bartosz Golaszewski (11):
configfs: increase the item name length
configfs: use (1UL << bit) for internal flags
configfs: implement committable items
samples: configfs: add a committable group
lib: bitmap: remove the 'extern' keyword from function declarations
lib: bitmap: order includes alphabetically
lib: bitmap: provide devm_bitmap_alloc() and devm_bitmap_zalloc()
gpio: sim: new testing module
selftests: gpio: provide a helper for reading chip info
selftests: gpio: add a helper for reading GPIO line names
selftests: gpio: add test cases for gpio-sim
Documentation/admin-guide/gpio/gpio-sim.rst | 72 ++
Documentation/filesystems/configfs.rst| 6 +-
drivers/gpio/Kconfig | 8 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-sim.c | 874 ++
fs/configfs/configfs_internal.h | 22 +-
fs/configfs/dir.c | 245 -
include/linux/bitmap.h| 127 +--
include/linux/configfs.h | 3 +-
lib/bitmap.c | 42 +-
samples/configfs/configfs_sample.c| 153 +++
tools/testing/selftests/gpio/.gitignore | 2 +
tools/testing/selftests/gpio/Makefile | 4 +-
tools/testing/selftests/gpio/config | 1 +
tools/testing/selftests/gpio/gpio-chip-info.c | 57 ++
tools/testing/selftests/gpio/gpio-line-name.c | 55 ++
tools/testing/selftests/gpio/gpio-sim.sh | 229 +
17 files changed, 1815 insertions(+), 86 deletions(-)
create mode 100644 Documentation/admin-guide/gpio/gpio-sim.rst
create mode 100644 drivers/gpio/gpio-sim.c
create mode 100644 tools/testing/selftests/gpio/gpio-chip-info.c
create mode 100644 tools/testing/selftests/gpio/gpio-line-name.c
create mode 100755 tools/testing/selftests/gpio/gpio-sim.sh
--
2.30.1
Re: [PATCH v3 08/11] gpio: sim: new testing module
On Wed, Mar 10, 2021 at 1:28 PM Andy Shevchenko
wrote:
>
[snip]
> > +
> > +static ssize_t gpio_sim_sysfs_line_show(struct device *dev,
> > + struct device_attribute *attr,
> > + char *buf)
> > +{
> > + struct gpio_sim_attribute *line_attr = to_gpio_sim_attr(attr);
> > + struct gpio_sim_chip *chip = dev_get_drvdata(dev);
> > + int ret;
> > +
> > + mutex_lock(>lock);
> > + ret = sprintf(buf, "%u\n", !!test_bit(line_attr->offset,
> > chip->values));
>
> Shouldn't we use sysfs_emit() in a new code?
>
TIL it exists. :) I'll use it.
[snip]
> > +
> > +static ssize_t gpio_sim_config_dev_name_show(struct config_item *item,
> > + char *page)
> > +{
> > + struct gpio_sim_chip_config *config = to_gpio_sim_chip_config(item);
> > + struct platform_device *pdev;
> > + int ret;
> > +
> > + mutex_lock(>lock);
> > + pdev = config->pdev;
> > + if (pdev)
> > + ret = sprintf(page, "%s\n", dev_name(>dev));
> > + else
> > + ret = sprintf(page, "n/a\n");
>
> I dunno '/' (slash) is a good character to be handled in a shell.
> I would prefer 'none' or 'not available' (I think space is easier,
> because the rules to escape much simpler: need just to take it into
> quotes, while / needs to be escaped separately).
>
My test cases work fine with 'n/a' but I can change it to 'none' if
it's less controversial.
[snip]
>
> Also don't know what the rules about using s*printf() in the configfs.
> Maybe we have sysfs_emit() analogue or it doesn't applicable here at all.
> Greg?
>
There's no configfs_emit() or anything similar. Output for simple
attributes must simply not exceed 4096 bytes. It used to be PAGE_SIZE,
now it's defined in fs/configfs/file.c as SIMPLE_ATTR_SIZE. There's no
need to check the length of the string here though as we're only
showing what we received from the user-space anyway and configfs makes
sure we don't get more than SIMPLE_ATTR_SIZE in the store callback.
[snip]
> > +
> > +static int gpio_sim_config_commit_item(struct config_item *item)
> > +{
> > + struct gpio_sim_chip_config *config = to_gpio_sim_chip_config(item);
> > + struct property_entry properties[GPIO_SIM_MAX_PROP];
> > + struct platform_device_info pdevinfo;
> > + struct platform_device *pdev;
> > + unsigned int prop_idx = 0;
> > +
> > + memset(, 0, sizeof(pdevinfo));
> > + memset(properties, 0, sizeof(properties));
> > +
> > + mutex_lock(>lock);
> > +
> > + properties[prop_idx++] = PROPERTY_ENTRY_U32("gpio-sim,nr-gpios",
> > + config->num_lines);
>
> > + if (config->label[0] != '\0')
>
> I'm wondering if we need this check. Isn't core taking care of it?
>
> > + properties[prop_idx++] =
> > PROPERTY_ENTRY_STRING("gpio-sim,label",
> > +config->label);
>
> > + if (config->line_names)
>
> Ditto.
>
> > + properties[prop_idx++] = PROPERTY_ENTRY_STRING_ARRAY_LEN(
> > + "gpio-line-names",
> > + config->line_names,
> > + config->num_line_names);
> > +
But I would be creating empty properties for nothing. Better to just
not have them at all.
[snip]
Bartosz
[PATCH v3 11/11] selftests: gpio: add test cases for gpio-sim
From: Bartosz Golaszewski
Add a set of tests for the new gpio-sim module. This is a pure shell
test-suite and uses the helper programs available in the gpio selftests
directory. These test-cases only test the functionalities exposed by the
gpio-sim driver, not those handled by core gpiolib code.
Signed-off-by: Bartosz Golaszewski
---
tools/testing/selftests/gpio/Makefile| 2 +-
tools/testing/selftests/gpio/config | 1 +
tools/testing/selftests/gpio/gpio-sim.sh | 229 +++
3 files changed, 231 insertions(+), 1 deletion(-)
create mode 100755 tools/testing/selftests/gpio/gpio-sim.sh
diff --git a/tools/testing/selftests/gpio/Makefile
b/tools/testing/selftests/gpio/Makefile
index d7d8f1985d99..4c6df61c76a8 100644
--- a/tools/testing/selftests/gpio/Makefile
+++ b/tools/testing/selftests/gpio/Makefile
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
-TEST_PROGS := gpio-mockup.sh
+TEST_PROGS := gpio-mockup.sh gpio-sim.sh
TEST_FILES := gpio-mockup-sysfs.sh
TEST_GEN_PROGS_EXTENDED := gpio-mockup-cdev gpio-chip-info gpio-line-name
diff --git a/tools/testing/selftests/gpio/config
b/tools/testing/selftests/gpio/config
index ce100342c20b..409a8532facc 100644
--- a/tools/testing/selftests/gpio/config
+++ b/tools/testing/selftests/gpio/config
@@ -1,3 +1,4 @@
CONFIG_GPIOLIB=y
CONFIG_GPIO_CDEV=y
CONFIG_GPIO_MOCKUP=m
+CONFIG_GPIO_SIM=m
diff --git a/tools/testing/selftests/gpio/gpio-sim.sh
b/tools/testing/selftests/gpio/gpio-sim.sh
new file mode 100755
index ..c8c6e4a7aebe
--- /dev/null
+++ b/tools/testing/selftests/gpio/gpio-sim.sh
@@ -0,0 +1,229 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2021 Bartosz Golaszewski
+
+BASE_DIR=`dirname $0`
+CONFIGFS_DIR="/sys/kernel/config/gpio-sim"
+PENDING_DIR=$CONFIGFS_DIR/pending
+LIVE_DIR=$CONFIGFS_DIR/live
+MODULE="gpio-sim"
+
+fail() {
+ echo "$*" >&2
+ echo "GPIO $MODULE test FAIL"
+ exit 1
+}
+
+skip() {
+ echo "$*" >&2
+ echo "GPIO $MODULE test SKIP"
+ exit 4
+}
+
+configfs_cleanup() {
+ for DIR in `ls $LIVE_DIR`; do
+ mv $LIVE_DIR/$DIR $PENDING_DIR
+ done
+
+ for DIR in `ls $PENDING_DIR`; do
+ rmdir $PENDING_DIR/$DIR
+ done
+}
+
+create_pending_chip() {
+ local NAME="$1"
+ local LABEL="$2"
+ local NUM_LINES="$3"
+ local LINE_NAMES="$4"
+ local CHIP_DIR="$PENDING_DIR/$NAME"
+
+ mkdir $CHIP_DIR
+ test -n "$LABEL" && echo $LABEL > $CHIP_DIR/label
+ test -n "$NUM_LINES" && echo $NUM_LINES > $CHIP_DIR/num_lines
+ if [ -n "$LINE_NAMES" ]; then
+ echo $LINE_NAMES 2> /dev/null > $CHIP_DIR/line_names
+ # This one can fail
+ if [ "$?" -ne "0" ]; then
+ return 1
+ fi
+ fi
+}
+
+create_live_chip() {
+ local CHIP_DIR="$PENDING_DIR/$1"
+
+ create_pending_chip "$@" || fail "unable to create the chip configfs
item"
+ mv $CHIP_DIR $LIVE_DIR || fail "unable to commit the chip configfs item"
+}
+
+remove_pending_chip() {
+ local NAME="$1"
+
+ rmdir $PENDING_DIR/$NAME || fail "unable to remove the chip configfs
item"
+}
+
+remove_live_chip() {
+ local NAME="$1"
+
+ mv $LIVE_DIR/$NAME $PENDING_DIR || fail "unable to uncommit the chip
configfs item"
+ remove_pending_chip "$@"
+}
+
+configfs_chip_name() {
+ local CHIP="$1"
+
+ cat $LIVE_DIR/$CHIP/chip_name 2> /dev/null || return 1
+}
+
+configfs_dev_name() {
+ local CHIP="$1"
+
+ cat $LIVE_DIR/$CHIP/dev_name 2> /dev/null || return 1
+}
+
+get_chip_num_lines() {
+ local CHIP="$1"
+
+ $BASE_DIR/gpio-chip-info /dev/`configfs_chip_name $CHIP` num-lines
+}
+
+get_chip_label() {
+ local CHIP="$1"
+
+ $BASE_DIR/gpio-chip-info /dev/`configfs_chip_name $CHIP` label
+}
+
+get_line_name() {
+ local CHIP="$1"
+ local OFFSET="$2"
+
+ $BASE_DIR/gpio-line-name /dev/`configfs_chip_name $CHIP` $OFFSET
+}
+
+sysfs_set_pull() {
+ local CHIP="$1"
+ local OFFSET="$2"
+ local PULL="$3"
+ local SYSFSPATH="/sys/devices/platform/`configfs_dev_name
$CHIP`/line-ctrl/gpio$OFFSET"
+
+ echo $PULL > $SYSFSPATH
+}
+
+# Load the gpio-sim module. This will pull in configfs if needed too.
+modprobe gpio-sim || skip "unable to load the gpio-sim module"
+# Make sure configfs is mounted at /sys/kernel/config. Wait a bit if needed.
+for IDX in `seq 5`; do
+ if [ "$IDX" -eq "5" ]; then
+ skip
[PATCH v3 10/11] selftests: gpio: add a helper for reading GPIO line names
From: Bartosz Golaszewski
Add a simple program that allows to read GPIO line names from the
character device. This will be used in gpio-sim selftests.
Signed-off-by: Bartosz Golaszewski
---
tools/testing/selftests/gpio/.gitignore | 1 +
tools/testing/selftests/gpio/Makefile | 2 +-
tools/testing/selftests/gpio/gpio-line-name.c | 55 +++
3 files changed, 57 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/gpio/gpio-line-name.c
diff --git a/tools/testing/selftests/gpio/.gitignore
b/tools/testing/selftests/gpio/.gitignore
index 4ea4f58dab1a..ededb077a3a6 100644
--- a/tools/testing/selftests/gpio/.gitignore
+++ b/tools/testing/selftests/gpio/.gitignore
@@ -1,3 +1,4 @@
# SPDX-License-Identifier: GPL-2.0-only
gpio-mockup-cdev
gpio-chip-info
+gpio-line-name
diff --git a/tools/testing/selftests/gpio/Makefile
b/tools/testing/selftests/gpio/Makefile
index 84b48547f94c..d7d8f1985d99 100644
--- a/tools/testing/selftests/gpio/Makefile
+++ b/tools/testing/selftests/gpio/Makefile
@@ -2,6 +2,6 @@
TEST_PROGS := gpio-mockup.sh
TEST_FILES := gpio-mockup-sysfs.sh
-TEST_GEN_PROGS_EXTENDED := gpio-mockup-cdev gpio-chip-info
+TEST_GEN_PROGS_EXTENDED := gpio-mockup-cdev gpio-chip-info gpio-line-name
include ../lib.mk
diff --git a/tools/testing/selftests/gpio/gpio-line-name.c
b/tools/testing/selftests/gpio/gpio-line-name.c
new file mode 100644
index ..a52e75bc37ba
--- /dev/null
+++ b/tools/testing/selftests/gpio/gpio-line-name.c
@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * GPIO character device helper for reading line names.
+ *
+ * Copyright (C) 2021 Bartosz Golaszewski
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+static void print_usage(void)
+{
+ printf("usage:\n");
+ printf(" gpio-line-name \n");
+}
+
+int main(int argc, char **argv)
+{
+ struct gpio_v2_line_info info;
+ int fd, ret;
+ char *endp;
+
+ if (argc != 3) {
+ print_usage();
+ return EXIT_FAILURE;
+ }
+
+ fd = open(argv[1], O_RDWR);
+ if (fd < 0) {
+ perror("unable to open the GPIO chip");
+ return EXIT_FAILURE;
+ }
+
+ memset(, 0, sizeof(info));
+ info.offset = strtoul(argv[2], , 10);
+ if (*endp != '\0') {
+ print_usage();
+ return EXIT_FAILURE;
+ }
+
+ ret = ioctl(fd, GPIO_V2_GET_LINEINFO_IOCTL, );
+ if (ret) {
+ perror("line info ioctl failed");
+ return EXIT_FAILURE;
+ }
+
+ printf("%s\n", info.name);
+
+ return EXIT_SUCCESS;
+}
--
2.30.1
[PATCH v3 09/11] selftests: gpio: provide a helper for reading chip info
From: Bartosz Golaszewski
Add a simple program that allows to retrieve chip properties from the
GPIO character device. This will be used in gpio-sim selftests.
Signed-off-by: Bartosz Golaszewski
---
tools/testing/selftests/gpio/.gitignore | 1 +
tools/testing/selftests/gpio/Makefile | 2 +-
tools/testing/selftests/gpio/gpio-chip-info.c | 57 +++
3 files changed, 59 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/gpio/gpio-chip-info.c
diff --git a/tools/testing/selftests/gpio/.gitignore
b/tools/testing/selftests/gpio/.gitignore
index a4969f7ee020..4ea4f58dab1a 100644
--- a/tools/testing/selftests/gpio/.gitignore
+++ b/tools/testing/selftests/gpio/.gitignore
@@ -1,2 +1,3 @@
# SPDX-License-Identifier: GPL-2.0-only
gpio-mockup-cdev
+gpio-chip-info
diff --git a/tools/testing/selftests/gpio/Makefile
b/tools/testing/selftests/gpio/Makefile
index 39f2bbe8dd3d..84b48547f94c 100644
--- a/tools/testing/selftests/gpio/Makefile
+++ b/tools/testing/selftests/gpio/Makefile
@@ -2,6 +2,6 @@
TEST_PROGS := gpio-mockup.sh
TEST_FILES := gpio-mockup-sysfs.sh
-TEST_GEN_PROGS_EXTENDED := gpio-mockup-cdev
+TEST_GEN_PROGS_EXTENDED := gpio-mockup-cdev gpio-chip-info
include ../lib.mk
diff --git a/tools/testing/selftests/gpio/gpio-chip-info.c
b/tools/testing/selftests/gpio/gpio-chip-info.c
new file mode 100644
index ..4d26fa7c254a
--- /dev/null
+++ b/tools/testing/selftests/gpio/gpio-chip-info.c
@@ -0,0 +1,57 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * GPIO character device helper for reading chip information.
+ *
+ * Copyright (C) 2021 Bartosz Golaszewski
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+static void print_usage(void)
+{
+ printf("usage:\n");
+ printf(" gpio-chip-info [name|label|num-lines]\n");
+}
+
+int main(int argc, char **argv)
+{
+ struct gpiochip_info info;
+ int fd, ret;
+
+ if (argc !=3) {
+ print_usage();
+ return EXIT_FAILURE;
+ }
+
+ fd = open(argv[1], O_RDWR);
+ if (fd < 0) {
+ perror("unable to open the GPIO chip");
+ return EXIT_FAILURE;
+ }
+
+ memset(, 0, sizeof(info));
+ ret = ioctl(fd, GPIO_GET_CHIPINFO_IOCTL, );
+ if (ret) {
+ perror("chip info ioctl failed");
+ return EXIT_FAILURE;
+ }
+
+ if (strcmp(argv[2], "name") == 0) {
+ printf("%s\n", info.name);
+ } else if (strcmp(argv[2], "label") == 0) {
+ printf("%s\n", info.label);
+ } else if (strcmp(argv[2], "num-lines") == 0) {
+ printf("%u\n", info.lines);
+ } else {
+ fprintf(stderr, "unknown command: %s\n", argv[2]);
+ return EXIT_FAILURE;
+ }
+
+ return EXIT_SUCCESS;
+}
--
2.30.1
[PATCH v3 08/11] gpio: sim: new testing module
From: Bartosz Golaszewski Implement a new, modern GPIO testing module controlled by configfs attributes instead of module parameters. The goal of this driver is to provide a replacement for gpio-mockup that will be easily extensible with new features and doesn't require reloading the module to change the setup. Signed-off-by: Bartosz Golaszewski --- Documentation/admin-guide/gpio/gpio-sim.rst | 72 ++ drivers/gpio/Kconfig| 8 + drivers/gpio/Makefile | 1 + drivers/gpio/gpio-sim.c | 879 4 files changed, 960 insertions(+) create mode 100644 Documentation/admin-guide/gpio/gpio-sim.rst create mode 100644 drivers/gpio/gpio-sim.c diff --git a/Documentation/admin-guide/gpio/gpio-sim.rst b/Documentation/admin-guide/gpio/gpio-sim.rst new file mode 100644 index ..08eac487e35e --- /dev/null +++ b/Documentation/admin-guide/gpio/gpio-sim.rst @@ -0,0 +1,72 @@ +.. SPDX-License-Identifier: GPL-2.0-or-later + +Configfs GPIO Simulator +=== + +The configfs GPIO Simulator (gpio-sim) provides a way to create simulated GPIO +chips for testing purposes. The lines exposed by these chips can be accessed +using the standard GPIO character device interface as well as manipulated +using sysfs attributes. + +Creating simulated chips + + +The gpio-sim module registers a configfs subsystem called 'gpio-sim'. It's a +subsystem with committable items which means two subdirectories are created in +the filesystem: pending and live. For more information on configfs and +committable items, please refer to Documentation/filesystems/configfs.rst. + +In order to instantiate a new simulated chip, the user needs to mkdir() a new +directory in pending/. Inside each new directory, there's a set of attributes +that can be used to configure the new chip. Once the configuration is complete, +the user needs to use rename() to move the chip to the live/ directory. This +creates and registers the new device. + +In order to destroy a simulated chip, it has to be moved back to pending first +and then removed using rmdir(). + +Currently supported configuration attributes are: + + num_lines - an unsigned integer value defining the number of GPIO lines to + export + + label - a string defining the label for the GPIO chip + + line_names - a list of GPIO line names in the form of quoted strings + separated by commas, e.g.: '"foo", "bar", "", "foobar"'. The + number of strings doesn't have to be equal to the value set in + the num_lines attribute. If it's lower than the number of lines, + the remaining lines are unnamed. If it's larger, the superfluous + lines are ignored. A name of the form: '""' means the line + should be unnamed. + +Additionally two read-only attributes named 'chip_name' and 'dev_name' are +exposed in order to provide users with a mapping from configfs directories to +the actual devices created in the kernel. The former returns the name of the +GPIO device as assigned by gpiolib (i.e. "gpiochip0", "gpiochip1", etc.). The +latter returns the parent device name as defined by the gpio-sim driver (i.e. +"gpio-sim.0", "gpio-sim.1", etc.). This allows user-space to map the configfs +items both to the correct character device file as well as the associated entry +in sysfs. + +Simulated GPIO chips can also be defined in device-tree. The compatible string +must be: "gpio-simulator". Supported properties are: + + "gpio-sim,label" - chip label + + "gpio-sim,nr-gpios" - number of lines + +Other standard GPIO properties (like "gpio-line-names" and gpio-hog) are also +supported. + +Manipulating simulated lines + + +Each simulated GPIO chip creates a sysfs attribute group under its device +directory called 'line-ctrl'. Inside each group, there's a separate attribute +for each GPIO line. The name of the attribute is of the form 'gpioX' where X +is the line's offset in the chip. + +Reading from a line attribute returns the current value. Writing to it (0 or 1) +changes the configuration of the simulated pull-up/pull-down resistor +(1 - pull-up, 0 - pull-down). diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index e3607ec4c2e8..b6b6150d0d04 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -1633,6 +1633,14 @@ config GPIO_MOCKUP tools/testing/selftests/gpio/gpio-mockup.sh. Reference the usage in it. +config GPIO_SIM + tristate "GPIO Simulator Module" + select IRQ_SIM + select CONFIGFS_FS + help + This enables the GPIO simulator - a configfs-based GPIO testing + driver. + endmenu endif diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index c
[PATCH v3 07/11] lib: bitmap: provide devm_bitmap_alloc() and devm_bitmap_zalloc()
From: Bartosz Golaszewski
Provide managed variants of bitmap_alloc() and bitmap_zalloc().
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Andy Shevchenko
---
include/linux/bitmap.h | 8
lib/bitmap.c | 33 +
2 files changed, 41 insertions(+)
diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index 3282db97e06c..73d039476fa4 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -9,6 +9,8 @@
#include
#include
+struct device;
+
/*
* bitmaps provide bit arrays that consume one or more unsigned
* longs. The bitmap interface and available operations are listed
@@ -122,6 +124,12 @@ unsigned long *bitmap_alloc(unsigned int nbits, gfp_t
flags);
unsigned long *bitmap_zalloc(unsigned int nbits, gfp_t flags);
void bitmap_free(const unsigned long *bitmap);
+/* Managed variants of the above. */
+unsigned long *devm_bitmap_alloc(struct device *dev,
+unsigned int nbits, gfp_t flags);
+unsigned long *devm_bitmap_zalloc(struct device *dev,
+ unsigned int nbits, gfp_t flags);
+
/*
* lib/bitmap.c provides these functions:
*/
diff --git a/lib/bitmap.c b/lib/bitmap.c
index 78f70d9007ad..b4fd7fd084c6 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -8,6 +8,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -1263,6 +1264,38 @@ void bitmap_free(const unsigned long *bitmap)
}
EXPORT_SYMBOL(bitmap_free);
+static void devm_bitmap_free(void *data)
+{
+ unsigned long *bitmap = data;
+
+ bitmap_free(bitmap);
+}
+
+unsigned long *devm_bitmap_alloc(struct device *dev,
+unsigned int nbits, gfp_t flags)
+{
+ unsigned long *bitmap;
+ int ret;
+
+ bitmap = bitmap_alloc(nbits, flags);
+ if (!bitmap)
+ return NULL;
+
+ ret = devm_add_action_or_reset(dev, devm_bitmap_free, bitmap);
+ if (ret)
+ return NULL;
+
+ return bitmap;
+}
+EXPORT_SYMBOL(devm_bitmap_alloc);
+
+unsigned long *devm_bitmap_zalloc(struct device *dev,
+ unsigned int nbits, gfp_t flags)
+{
+ return devm_bitmap_alloc(dev, nbits, flags | __GFP_ZERO);
+}
+EXPORT_SYMBOL(devm_bitmap_zalloc);
+
#if BITS_PER_LONG == 64
/**
* bitmap_from_arr32 - copy the contents of u32 array of bits to bitmap
--
2.30.1
[PATCH v3 06/11] lib: bitmap: order includes alphabetically
From: Bartosz Golaszewski For better readability and maintenance: order the includes in bitmap source files alphabetically. Signed-off-by: Bartosz Golaszewski Reviewed-by: Andy Shevchenko --- include/linux/bitmap.h | 4 ++-- lib/bitmap.c | 9 + 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h index 6939a8983026..3282db97e06c 100644 --- a/include/linux/bitmap.h +++ b/include/linux/bitmap.h @@ -4,10 +4,10 @@ #ifndef __ASSEMBLY__ -#include #include -#include #include +#include +#include /* * bitmaps provide bit arrays that consume one or more unsigned diff --git a/lib/bitmap.c b/lib/bitmap.c index 75006c4036e9..78f70d9007ad 100644 --- a/lib/bitmap.c +++ b/lib/bitmap.c @@ -3,17 +3,18 @@ * lib/bitmap.c * Helper functions for bitmap.h. */ -#include -#include -#include -#include + #include #include #include +#include +#include +#include #include #include #include #include +#include #include #include -- 2.30.1
[PATCH v3 05/11] lib: bitmap: remove the 'extern' keyword from function declarations
From: Bartosz Golaszewski The 'extern' keyword doesn't have any benefits in header files. Remove it. Signed-off-by: Bartosz Golaszewski Reviewed-by: Andy Shevchenko --- include/linux/bitmap.h | 115 - 1 file changed, 57 insertions(+), 58 deletions(-) diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h index 70a932470b2d..6939a8983026 100644 --- a/include/linux/bitmap.h +++ b/include/linux/bitmap.h @@ -118,54 +118,53 @@ * Allocation and deallocation of bitmap. * Provided in lib/bitmap.c to avoid circular dependency. */ -extern unsigned long *bitmap_alloc(unsigned int nbits, gfp_t flags); -extern unsigned long *bitmap_zalloc(unsigned int nbits, gfp_t flags); -extern void bitmap_free(const unsigned long *bitmap); +unsigned long *bitmap_alloc(unsigned int nbits, gfp_t flags); +unsigned long *bitmap_zalloc(unsigned int nbits, gfp_t flags); +void bitmap_free(const unsigned long *bitmap); /* * lib/bitmap.c provides these functions: */ -extern int __bitmap_equal(const unsigned long *bitmap1, - const unsigned long *bitmap2, unsigned int nbits); -extern bool __pure __bitmap_or_equal(const unsigned long *src1, -const unsigned long *src2, -const unsigned long *src3, -unsigned int nbits); -extern void __bitmap_complement(unsigned long *dst, const unsigned long *src, - unsigned int nbits); -extern void __bitmap_shift_right(unsigned long *dst, const unsigned long *src, - unsigned int shift, unsigned int nbits); -extern void __bitmap_shift_left(unsigned long *dst, const unsigned long *src, - unsigned int shift, unsigned int nbits); -extern void bitmap_cut(unsigned long *dst, const unsigned long *src, - unsigned int first, unsigned int cut, - unsigned int nbits); -extern int __bitmap_and(unsigned long *dst, const unsigned long *bitmap1, +int __bitmap_equal(const unsigned long *bitmap1, + const unsigned long *bitmap2, unsigned int nbits); +bool __pure __bitmap_or_equal(const unsigned long *src1, + const unsigned long *src2, + const unsigned long *src3, + unsigned int nbits); +void __bitmap_complement(unsigned long *dst, const unsigned long *src, +unsigned int nbits); +void __bitmap_shift_right(unsigned long *dst, const unsigned long *src, + unsigned int shift, unsigned int nbits); +void __bitmap_shift_left(unsigned long *dst, const unsigned long *src, +unsigned int shift, unsigned int nbits); +void bitmap_cut(unsigned long *dst, const unsigned long *src, + unsigned int first, unsigned int cut, unsigned int nbits); +int __bitmap_and(unsigned long *dst, const unsigned long *bitmap1, +const unsigned long *bitmap2, unsigned int nbits); +void __bitmap_or(unsigned long *dst, const unsigned long *bitmap1, +const unsigned long *bitmap2, unsigned int nbits); +void __bitmap_xor(unsigned long *dst, const unsigned long *bitmap1, + const unsigned long *bitmap2, unsigned int nbits); +int __bitmap_andnot(unsigned long *dst, const unsigned long *bitmap1, + const unsigned long *bitmap2, unsigned int nbits); +void __bitmap_replace(unsigned long *dst, + const unsigned long *old, const unsigned long *new, + const unsigned long *mask, unsigned int nbits); +int __bitmap_intersects(const unsigned long *bitmap1, const unsigned long *bitmap2, unsigned int nbits); -extern void __bitmap_or(unsigned long *dst, const unsigned long *bitmap1, - const unsigned long *bitmap2, unsigned int nbits); -extern void __bitmap_xor(unsigned long *dst, const unsigned long *bitmap1, - const unsigned long *bitmap2, unsigned int nbits); -extern int __bitmap_andnot(unsigned long *dst, const unsigned long *bitmap1, - const unsigned long *bitmap2, unsigned int nbits); -extern void __bitmap_replace(unsigned long *dst, - const unsigned long *old, const unsigned long *new, - const unsigned long *mask, unsigned int nbits); -extern int __bitmap_intersects(const unsigned long *bitmap1, - const unsigned long *bitmap2, unsigned int nbits); -extern int __bitmap_subset(const unsigned long *bitmap1, - const unsigned long *bitmap2, unsigned int nbits); -extern int __bitmap_weight(const unsigned long *bitmap, unsigned int nbits); -extern void __bitmap_set(unsigned long *map, unsigned int start, int len); -extern void __bitmap_clear(unsigned long *map, unsigned int start, int len); - -extern unsigned
[PATCH v3 01/11] configfs: increase the item name length
From: Bartosz Golaszewski 20 characters limit for item name is relatively small. Let's increase it to 32 to fit '04-committable-children' - a name we'll use in the sample code for committable items. Signed-off-by: Bartosz Golaszewski Acked-by: Linus Walleij --- include/linux/configfs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/configfs.h b/include/linux/configfs.h index 2e8c69b43c64..4f76dcc08134 100644 --- a/include/linux/configfs.h +++ b/include/linux/configfs.h @@ -27,7 +27,7 @@ #include/* struct kref */ #include /* struct mutex */ -#define CONFIGFS_ITEM_NAME_LEN 20 +#define CONFIGFS_ITEM_NAME_LEN 32 struct module; -- 2.30.1
[PATCH v3 04/11] samples: configfs: add a committable group
From: Bartosz Golaszewski
Add an example of using committable items to configfs samples. Each
config item has two attributes: read-write 'storeme' which works
similarly to other examples in this file and a read-only 'committed'
attribute which changes its value between false and true depending on
whether it's committed or not at the moment.
Signed-off-by: Bartosz Golaszewski
Acked-by: Linus Walleij
---
samples/configfs/configfs_sample.c | 153 +
1 file changed, 153 insertions(+)
diff --git a/samples/configfs/configfs_sample.c
b/samples/configfs/configfs_sample.c
index f9008be7a8a1..9bef74e4369d 100644
--- a/samples/configfs/configfs_sample.c
+++ b/samples/configfs/configfs_sample.c
@@ -315,6 +315,158 @@ static struct configfs_subsystem group_children_subsys = {
/* - */
+/*
+ * 04-committable-children
+ *
+ * This is an example of a committable group. It's similar to the simple
+ * children example but each config_item has an additional 'committed'
+ * attribute which is read-only and is only modified when the config_item
+ * is moved from the 'pending' to the 'live' directory.
+ */
+
+struct committable_child {
+ struct config_item item;
+ int storeme;
+ bool committed;
+};
+
+static inline struct committable_child *
+to_committable_child(struct config_item *item)
+{
+ return container_of(item, struct committable_child, item);
+}
+
+static ssize_t
+committable_child_storeme_show(struct config_item *item, char *page)
+{
+ return sprintf(page, "%d\n", to_committable_child(item)->storeme);
+}
+
+static ssize_t committable_child_storeme_store(struct config_item *item,
+ const char *page, size_t count)
+{
+ struct committable_child *child = to_committable_child(item);
+ int ret;
+
+ if (child->committed)
+ return -EPERM;
+
+ ret = kstrtoint(page, 10, >storeme);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+CONFIGFS_ATTR(committable_child_, storeme);
+
+static ssize_t
+committable_child_committed_show(struct config_item *item, char *page)
+{
+ return sprintf(page, "%s\n",
+ to_committable_child(item)->committed ? "true" : "false");
+}
+
+CONFIGFS_ATTR_RO(committable_child_, committed);
+
+static struct configfs_attribute *committable_child_attrs[] = {
+ _child_attr_storeme,
+ _child_attr_committed,
+ NULL,
+};
+
+static void committable_child_release(struct config_item *item)
+{
+ kfree(to_committable_child(item));
+}
+
+static struct configfs_item_operations committable_child_item_ops = {
+ .release= committable_child_release,
+};
+
+static const struct config_item_type committable_child_type = {
+ .ct_item_ops= _child_item_ops,
+ .ct_attrs = committable_child_attrs,
+ .ct_owner = THIS_MODULE,
+};
+
+struct committable_children {
+ struct config_group group;
+};
+
+static struct config_item *
+committable_children_make_item(struct config_group *group, const char *name)
+{
+ struct committable_child *child;
+
+ child = kzalloc(sizeof(*child), GFP_KERNEL);
+ if (!child)
+ return ERR_PTR(-ENOMEM);
+
+ config_item_init_type_name(>item, name, _child_type);
+
+ return >item;
+}
+
+static ssize_t
+committable_children_description_show(struct config_item *item, char *page)
+{
+ return sprintf(page,
+"[04-committable-children]\n"
+"\n"
+"This subsystem allows creation of committable config_items. The subsystem\n"
+"has two subdirectories: pending and live. New config_items can only be\n"
+"created in pending/ and they have one writable and readable attribute as\n"
+"well as a single read-only attribute. The latter is only changed once the\n"
+"item is 'committed'. This is done by moving the config_item (using\n"
+"rename()) to the live/ directory. In this example, the storeme attribute\n"
+"becomes 'read-only' once committed.\n");
+}
+
+CONFIGFS_ATTR_RO(committable_children_, description);
+
+static struct configfs_attribute *committable_children_attrs[] = {
+ _children_attr_description,
+ NULL,
+};
+
+static int committable_children_commit_item(struct config_item *item)
+{
+ to_committable_child(item)->committed = true;
+
+ return 0;
+}
+
+static int committable_children_uncommit_item(struct config_item *item)
+{
+ to_committable_child(item)->committed = false;
+
+ return 0;
+}
+
+static struct configfs_group_operations committable_children_group_ops = {
+ .make_item = committable_children_make_item,
+ .commit_item= committable_children_commit_item,
+ .uncommit_item = committable_children
[PATCH v3 02/11] configfs: use (1UL << bit) for internal flags
From: Bartosz Golaszewski
For better readability and maintenance: use the (1UL << bit) for flag
definitions.
Signed-off-by: Bartosz Golaszewski
Acked-by: Linus Walleij
---
fs/configfs/configfs_internal.h | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/fs/configfs/configfs_internal.h b/fs/configfs/configfs_internal.h
index 9a3aed249692..b495c9f043d4 100644
--- a/fs/configfs/configfs_internal.h
+++ b/fs/configfs/configfs_internal.h
@@ -46,16 +46,16 @@ struct configfs_dirent {
struct configfs_fragment *s_frag;
};
-#define CONFIGFS_ROOT 0x0001
-#define CONFIGFS_DIR 0x0002
-#define CONFIGFS_ITEM_ATTR 0x0004
-#define CONFIGFS_ITEM_BIN_ATTR 0x0008
-#define CONFIGFS_ITEM_LINK 0x0020
-#define CONFIGFS_USET_DIR 0x0040
-#define CONFIGFS_USET_DEFAULT 0x0080
-#define CONFIGFS_USET_DROPPING 0x0100
-#define CONFIGFS_USET_IN_MKDIR 0x0200
-#define CONFIGFS_USET_CREATING 0x0400
+#define CONFIGFS_ROOT (1UL << 0)
+#define CONFIGFS_DIR (1UL << 1)
+#define CONFIGFS_ITEM_ATTR (1UL << 2)
+#define CONFIGFS_ITEM_BIN_ATTR (1UL << 3)
+#define CONFIGFS_ITEM_LINK (1UL << 5)
+#define CONFIGFS_USET_DIR (1UL << 6)
+#define CONFIGFS_USET_DEFAULT (1UL << 7)
+#define CONFIGFS_USET_DROPPING (1UL << 8)
+#define CONFIGFS_USET_IN_MKDIR (1UL << 9)
+#define CONFIGFS_USET_CREATING (1UL << 10)
#define CONFIGFS_NOT_PINNED(CONFIGFS_ITEM_ATTR | CONFIGFS_ITEM_BIN_ATTR)
extern struct mutex configfs_symlink_mutex;
--
2.30.1
[PATCH v3 03/11] configfs: implement committable items
From: Bartosz Golaszewski
This implements configfs committable items. We mostly follow the
documentation except that we extend config_group_ops with uncommit_item()
callback for reverting the changes made by commit_item().
Each committable group has two sub-directories: pending and live. New
items can only be created in pending/. Attributes can only be modified
while the item is in pending/. Once it's ready to be committed, it must
be moved over to live/ using the rename() system call. This is when the
commit_item() function will be called.
Implementation-wise: we reuse the default group mechanism to elegantly
plug the new pseude-groups into configfs. The pending group inherits the
parent group's operations so that config_items can be seamlesly created
in it using the callbacks supplied by the user as part of the committable
group itself.
Signed-off-by: Bartosz Golaszewski
Acked-by: Linus Walleij
---
Documentation/filesystems/configfs.rst | 6 +-
fs/configfs/configfs_internal.h| 2 +
fs/configfs/dir.c | 245 -
include/linux/configfs.h | 1 +
4 files changed, 245 insertions(+), 9 deletions(-)
diff --git a/Documentation/filesystems/configfs.rst
b/Documentation/filesystems/configfs.rst
index 1d3d6f4a82a9..7e0e7c356450 100644
--- a/Documentation/filesystems/configfs.rst
+++ b/Documentation/filesystems/configfs.rst
@@ -290,6 +290,7 @@ config_item_type::
struct config_group *(*make_group)(struct config_group *group,
const char *name);
int (*commit_item)(struct config_item *item);
+ int (*uncommit_item)(struct config_item *item);
void (*disconnect_notify)(struct config_group *group,
struct config_item *item);
void (*drop_item)(struct config_group *group,
@@ -490,9 +491,6 @@ pass up an error.
Committable Items
=
-Note:
- Committable items are currently unimplemented.
-
Some config_items cannot have a valid initial state. That is, no
default values can be specified for the item's attributes such that the
item can do its work. Userspace must configure one or more attributes,
@@ -532,4 +530,4 @@ method returns zero and the item is moved to the "live"
directory.
As rmdir(2) does not work in the "live" directory, an item must be
shutdown, or "uncommitted". Again, this is done via rename(2), this
time from the "live" directory back to the "pending" one. The subsystem
-is notified by the ct_group_ops->uncommit_object() method.
+is notified by the ct_group_ops->uncommit_item() method.
diff --git a/fs/configfs/configfs_internal.h b/fs/configfs/configfs_internal.h
index b495c9f043d4..41ac21c82bf5 100644
--- a/fs/configfs/configfs_internal.h
+++ b/fs/configfs/configfs_internal.h
@@ -56,6 +56,8 @@ struct configfs_dirent {
#define CONFIGFS_USET_DROPPING (1UL << 8)
#define CONFIGFS_USET_IN_MKDIR (1UL << 9)
#define CONFIGFS_USET_CREATING (1UL << 10)
+#define CONFIGFS_GROUP_PENDING (1UL << 11)
+#define CONFIGFS_GROUP_LIVE(1UL << 12)
#define CONFIGFS_NOT_PINNED(CONFIGFS_ITEM_ATTR | CONFIGFS_ITEM_BIN_ATTR)
extern struct mutex configfs_symlink_mutex;
diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
index b6098e02e20b..f3c95c1d5278 100644
--- a/fs/configfs/dir.c
+++ b/fs/configfs/dir.c
@@ -656,6 +656,13 @@ static void detach_groups(struct config_group *group)
inode_unlock(d_inode(child));
+ /*
+* Free memory allocated for the pending and live directories
+* of committable groups.
+*/
+ if (sd->s_type & (CONFIGFS_GROUP_PENDING | CONFIGFS_GROUP_LIVE))
+ kfree(sd->s_element);
+
d_delete(child);
dput(child);
}
@@ -860,6 +867,134 @@ static void configfs_detach_item(struct config_item *item)
configfs_remove_dir(item);
}
+static bool is_committable_group(struct config_item *item)
+{
+ const struct config_item_type *type = item->ci_type;
+
+ if (type && type->ct_group_ops &&
+ type->ct_group_ops->commit_item &&
+ type->ct_group_ops->uncommit_item)
+ return true;
+
+ return false;
+}
+
+struct pending_group_data {
+ struct config_group group;
+ struct config_item_type type;
+ struct configfs_group_operations group_ops;
+};
+
+struct live_group_data {
+ struct config_group group;
+ struct config_item_type type;
+};
+
+static int create_pending_group(struct config_item *parent_item,
+ struct configfs_fragment *frag)
+{
+ const struct config_item_type *parent_type = parent_item->
[PATCH v3 00/11] gpio: implement the configfs testing module
From: Bartosz Golaszewski
This series adds a new GPIO testing module based on configfs committable items
and sysfs. The goal is to provide a testing driver that will be configurable
at runtime (won't need module reload) and easily extensible. The control over
the attributes is also much more fine-grained than in gpio-mockup.
This series also contains a respin of the patches I sent separately to the
configfs maintainers - these patches implement the concept of committable
items that was well defined for a long time but never actually completed.
Apart from the new driver itself, its selftests and the configfs patches, this
series contains some changes to the bitmap API - most importantly: it adds
devres managed variants of bitmap_alloc() and bitmap_zalloc().
v1 -> v2:
- add selftests for gpio-sim
- add helper programs for selftests
- update the configfs rename callback to work with the new API introduced in
v5.11
- fix a missing quote in the documentation
- use !! whenever using bits operation that are required to return 0 or 1
- use provided bitmap API instead of reimplementing copy or fill operations
- fix a deadlock in gpio_sim_direction_output()
- add new read-only configfs attributes for mapping of configfs items to GPIO
device names
- and address other minor issues pointed out in reviews of v1
v2 -> v3:
- use devm_bitmap_alloc() instead of the zalloc variant if we're initializing
the bitmap with 1s
- drop the patch exporting device_is_bound()
- don't return -ENODEV from dev_nam and chip_name configfs attributes, return
a string indicating that the device is not available yet ('n/a')
- fix indentation where it makes sense
- don't protect IDA functions which use their own locking and where it's not
needed
- use kmemdup() instead of kzalloc() + memcpy()
- collected review tags
- minor coding style fixes
Bartosz Golaszewski (11):
configfs: increase the item name length
configfs: use (1UL << bit) for internal flags
configfs: implement committable items
samples: configfs: add a committable group
lib: bitmap: remove the 'extern' keyword from function declarations
lib: bitmap: order includes alphabetically
lib: bitmap: provide devm_bitmap_alloc() and devm_bitmap_zalloc()
gpio: sim: new testing module
selftests: gpio: provide a helper for reading chip info
selftests: gpio: add a helper for reading GPIO line names
selftests: gpio: add test cases for gpio-sim
Documentation/admin-guide/gpio/gpio-sim.rst | 72 ++
Documentation/filesystems/configfs.rst| 6 +-
drivers/gpio/Kconfig | 8 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-sim.c | 879 ++
fs/configfs/configfs_internal.h | 22 +-
fs/configfs/dir.c | 245 -
include/linux/bitmap.h| 127 +--
include/linux/configfs.h | 3 +-
lib/bitmap.c | 42 +-
samples/configfs/configfs_sample.c| 153 +++
tools/testing/selftests/gpio/.gitignore | 2 +
tools/testing/selftests/gpio/Makefile | 4 +-
tools/testing/selftests/gpio/config | 1 +
tools/testing/selftests/gpio/gpio-chip-info.c | 57 ++
tools/testing/selftests/gpio/gpio-line-name.c | 55 ++
tools/testing/selftests/gpio/gpio-sim.sh | 229 +
17 files changed, 1820 insertions(+), 86 deletions(-)
create mode 100644 Documentation/admin-guide/gpio/gpio-sim.rst
create mode 100644 drivers/gpio/gpio-sim.c
create mode 100644 tools/testing/selftests/gpio/gpio-chip-info.c
create mode 100644 tools/testing/selftests/gpio/gpio-line-name.c
create mode 100755 tools/testing/selftests/gpio/gpio-sim.sh
--
2.30.1
Re: [GIT PULL] gpio: fixes for v5.12-rc3
On Tue, Mar 9, 2021 at 4:34 PM Bartosz Golaszewski wrote: > > Linus, > > Here's a bunch of fixes for the GPIO subsystem. We have two regressions in the > core code spotted right after the merge window, a series of fixes for ACPI > GPIO > and a subsequent fix for a related regression in gpio-pca953x + a minor tweak > in .gitignore and a rework of handling of the gpio-line-names to remedy a > regression in stm32mp151. > > Please pull! > > Best Regards, > Bartosz Golaszewski > > The following changes since commit a38fd8748464831584a19438cbb3082b5a2dab15: > > Linux 5.12-rc2 (2021-03-05 17:33:41 -0800) > > are available in the Git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git > tags/gpio-fixes-for-v5.12-rc3 > > for you to fetch changes up to b41ba2ec54a70908067034f139aa23d0dd2985ce: > > gpiolib: Read "gpio-line-names" from a firmware node (2021-03-08 11:59:17 > +0100) > > > gpio fixes for v5.12-rc3 > > - fix two regressions in core GPIO subsystem code: one NULL-pointer > dereference > and one list corruption > - read GPIO line names from fwnode instead of using the generic device > properties to fix a regression on stm32mp151 > - fixes to ACPI GPIO and gpio-pca953x to handle a regression in IRQ handling > on Intel Galileo > - update .gitignore in GPIO selftests > > > Andy Shevchenko (4): > gpiolib: acpi: Add ACPI_GPIO_QUIRK_ABSOLUTE_NUMBER quirk > gpiolib: acpi: Allow to find GpioInt() resource by name and index > gpio: pca953x: Set IRQ type when handle Intel Galileo Gen 2 > gpiolib: Read "gpio-line-names" from a firmware node > > Bartosz Golaszewski (1): > selftests: gpio: update .gitignore > > Johan Hovold (2): > gpio: fix NULL-deref-on-deregistration regression > gpio: fix gpio-device list corruption > > Yang Li (1): > gpiolib: acpi: Add missing IRQF_ONESHOT > > drivers/gpio/gpio-pca953x.c | 78 > ++--- > drivers/gpio/gpiolib-acpi.c | 21 ++--- > drivers/gpio/gpiolib.c | 19 > include/linux/acpi.h| 10 - > include/linux/gpio/consumer.h | 2 + > tools/testing/selftests/gpio/.gitignore | 2 +- > 6 files changed, 58 insertions(+), 74 deletions(-) Hi Linus, I realized only after I sent out this PR that I had rebased the branch on top of v5.12-rc2 (because of the v5.12-rc1 situation) without --rebase-merges and this caused git to drop the merge commit for Andy's pull-request. Please let me know if you can pull this as is or if I should rebuild my branch and resend. Best Regards, Bartosz Golaszewski
[GIT PULL] gpio: fixes for v5.12-rc3
Linus, Here's a bunch of fixes for the GPIO subsystem. We have two regressions in the core code spotted right after the merge window, a series of fixes for ACPI GPIO and a subsequent fix for a related regression in gpio-pca953x + a minor tweak in .gitignore and a rework of handling of the gpio-line-names to remedy a regression in stm32mp151. Please pull! Best Regards, Bartosz Golaszewski The following changes since commit a38fd8748464831584a19438cbb3082b5a2dab15: Linux 5.12-rc2 (2021-03-05 17:33:41 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git tags/gpio-fixes-for-v5.12-rc3 for you to fetch changes up to b41ba2ec54a70908067034f139aa23d0dd2985ce: gpiolib: Read "gpio-line-names" from a firmware node (2021-03-08 11:59:17 +0100) gpio fixes for v5.12-rc3 - fix two regressions in core GPIO subsystem code: one NULL-pointer dereference and one list corruption - read GPIO line names from fwnode instead of using the generic device properties to fix a regression on stm32mp151 - fixes to ACPI GPIO and gpio-pca953x to handle a regression in IRQ handling on Intel Galileo - update .gitignore in GPIO selftests Andy Shevchenko (4): gpiolib: acpi: Add ACPI_GPIO_QUIRK_ABSOLUTE_NUMBER quirk gpiolib: acpi: Allow to find GpioInt() resource by name and index gpio: pca953x: Set IRQ type when handle Intel Galileo Gen 2 gpiolib: Read "gpio-line-names" from a firmware node Bartosz Golaszewski (1): selftests: gpio: update .gitignore Johan Hovold (2): gpio: fix NULL-deref-on-deregistration regression gpio: fix gpio-device list corruption Yang Li (1): gpiolib: acpi: Add missing IRQF_ONESHOT drivers/gpio/gpio-pca953x.c | 78 ++--- drivers/gpio/gpiolib-acpi.c | 21 ++--- drivers/gpio/gpiolib.c | 19 include/linux/acpi.h| 10 - include/linux/gpio/consumer.h | 2 + tools/testing/selftests/gpio/.gitignore | 2 +- 6 files changed, 58 insertions(+), 74 deletions(-)
Re: [PATCH v2 05/12] lib: bitmap: remove the 'extern' keyword from function declarations
On Thu, Mar 4, 2021 at 1:59 PM Andy Shevchenko wrote: > > On Thu, Mar 04, 2021 at 11:24:45AM +0100, Bartosz Golaszewski wrote: > > From: Bartosz Golaszewski > > > > The 'extern' keyword doesn't have any benefits in header files. Remove it. > > Reviewed-by: Andy Shevchenko > A few nitpicks below. > > > Signed-off-by: Bartosz Golaszewski > > --- Hi Andy, regarding this patch and other places where you raise issues with line breaking: I believe this is purely a question of taste. There are no guidelines on line breaking in the docs. I will leave it as it is here because it's not better or worse than your version, just different. Same for exceeding 80 characters - I personally believe it's justified when the line looks better but whenever it can be cleanly broken, it's better to stay within the limit. Best Regards, Bartosz
Re: [PATCH v3 0/5] gpiolib: switch to fwnode in the core
On Mon, Mar 8, 2021 at 8:52 PM Andy Shevchenko wrote: > > On Mon, Mar 08, 2021 at 09:36:52PM +0200, Andy Shevchenko wrote: > > On Mon, Mar 08, 2021 at 08:29:27PM +0100, Bartosz Golaszewski wrote: > > > On Mon, Mar 8, 2021 at 8:26 PM Rafael J. Wysocki > > > wrote: > > > > On Mon, Mar 8, 2021 at 8:23 PM Bartosz Golaszewski > > > > wrote: > > > > ... > > > > > > My impression was that Andy wanted me to take them. > > > > > > > > However, if you'd rather take care of them yourself, there you go! > > > > > > > > I'll drop them now and assume that they will be routed through the GPIO > > > > tree. > > > > > > > > Thanks! > > > > > > They touch a lot of core GPIO code and are likely to conflict if any > > > other changes show up this release cycle. I'd rather take them through > > > the usual channel. Thanks! > > > > Since now we have v4 based on Rafael's bleeding-edge, what do you want me to > > do? Resend a v5 with all patches included? > > I have decided to resend as usually it's better for maintainers. > > But it appears I was too quick to miss Rafael's review tag / comments. > > So, I will send v6 with those included. > > -- > With Best Regards, > Andy Shevchenko > > Does this series depend on patches already in Rafael's tree? If so, maybe Rafael can provide me with an immutable tag to merge in? Bartosz
Re: [PATCH v3 0/5] gpiolib: switch to fwnode in the core
On Mon, Mar 8, 2021 at 8:26 PM Rafael J. Wysocki wrote: > > On Mon, Mar 8, 2021 at 8:23 PM Bartosz Golaszewski > wrote: > > > > On Mon, Mar 8, 2021 at 7:22 PM Rafael J. Wysocki wrote: > > > > > > On Thu, Mar 4, 2021 at 9:13 PM Andy Shevchenko > > > wrote: > > > > > > > > GPIO library uses of_node and fwnode in the core in non-unified way. > > > > The series cleans this up and improves IRQ domain creation for non-OF > > > > cases > > > > where currently the names of the domain are 'unknown'. > > > > > > > > This has been tested on Intel Galileo Gen 2. > > > > > > > > In v3: > > > > - fix subtle bug in gpiod_count > > > > - make irq_domain_add_simple() static inline (Marc) > > > > > > > > In v2: > > > > - added a new patch due to functionality in irq_comain_add_simple() > > > > (Linus) > > > > - tagged patches 2-4 (Linus) > > > > - Cc'ed to Rafael > > > > > > > > Andy Shevchenko (5): > > > > irqdomain: Introduce irq_domain_create_simple() API > > > > gpiolib: Unify the checks on fwnode type > > > > gpiolib: Move of_node operations to gpiolib-of and correct fwnode use > > > > gpiolib: Introduce acpi_gpio_dev_init() and call it from core > > > > gpiolib: Reuse device's fwnode to create IRQ domain > > > > > > [1-4/5] applied as 5.13 material and I have a minor comment regarding > > > the last patch (will send separately). > > > > > > Thanks! > > > > Hi Rafael! > > > > AFAICT this should go through the GPIO tree as usual. Any reason for > > you to pick these patches this time? > > My impression was that Andy wanted me to take them. > > However, if you'd rather take care of them yourself, there you go! > > I'll drop them now and assume that they will be routed through the GPIO tree. > > Thanks! They touch a lot of core GPIO code and are likely to conflict if any other changes show up this release cycle. I'd rather take them through the usual channel. Thanks! Bartosz
Re: [PATCH v3 0/5] gpiolib: switch to fwnode in the core
On Mon, Mar 8, 2021 at 7:22 PM Rafael J. Wysocki wrote: > > On Thu, Mar 4, 2021 at 9:13 PM Andy Shevchenko > wrote: > > > > GPIO library uses of_node and fwnode in the core in non-unified way. > > The series cleans this up and improves IRQ domain creation for non-OF cases > > where currently the names of the domain are 'unknown'. > > > > This has been tested on Intel Galileo Gen 2. > > > > In v3: > > - fix subtle bug in gpiod_count > > - make irq_domain_add_simple() static inline (Marc) > > > > In v2: > > - added a new patch due to functionality in irq_comain_add_simple() (Linus) > > - tagged patches 2-4 (Linus) > > - Cc'ed to Rafael > > > > Andy Shevchenko (5): > > irqdomain: Introduce irq_domain_create_simple() API > > gpiolib: Unify the checks on fwnode type > > gpiolib: Move of_node operations to gpiolib-of and correct fwnode use > > gpiolib: Introduce acpi_gpio_dev_init() and call it from core > > gpiolib: Reuse device's fwnode to create IRQ domain > > [1-4/5] applied as 5.13 material and I have a minor comment regarding > the last patch (will send separately). > > Thanks! Hi Rafael! AFAICT this should go through the GPIO tree as usual. Any reason for you to pick these patches this time? Bartosz
Re: [PATCH v1 1/2] lib/cmdline: Export next_arg() for being used in modules
On Thu, Mar 4, 2021 at 12:01 PM Andy Shevchenko wrote: > > On Thu, Mar 04, 2021 at 09:53:28AM +0100, Geert Uytterhoeven wrote: > > On Mon, Mar 1, 2021 at 6:00 PM Andy Shevchenko > > wrote: > > > At least one module will benefit from using next_arg() helper. > > > Let's export it for that module and others if they consider it > > > helpful. > > > > > > Signed-off-by: Andy Shevchenko > > > > Reviewed-by: Geert Uytterhoeven > > Thanks Geert and Linus! > > Bart, do you want me to add it to my usual PR or you want to take it yourself > (I have no dependencies in my tree on this anyway)? > > -- > With Best Regards, > Andy Shevchenko > > Please send it with your PR. Bart
Re: [PATCH v2 09/12] gpio: sim: new testing module
On Mon, Mar 8, 2021 at 4:32 PM Andy Shevchenko wrote: > > On Mon, Mar 08, 2021 at 04:13:33PM +0100, Bartosz Golaszewski wrote: > > On Mon, Mar 8, 2021 at 4:05 PM Andy Shevchenko > > wrote: > > > On Mon, Mar 08, 2021 at 03:23:31PM +0100, Bartosz Golaszewski wrote: > > ... > > > > I have strong opinion not to open code "yet another parser". > > > > > > So, grepping on 'strsep(.*, ",")' shows a lot of code that wants > > > something like > > > this. Interesting are the net/9p cases. This in particular pointed out to > > > lib/parser.c which in turn shows promising match_strlcpy() / > > > match_strdup(). I > > > haven't looked deeply though. > > > > > > That said, I agree that next_arg() is not the best here. > > > > Shall we revisit this once it's upstream with a generalization for > > separating comma separated strings? > > How can we guarantee it won't be forgotten? > I will add a REVISIT comment, so *obviously* it ***will*** be revisited. :) Bartosz
Re: [PATCH] docs: driver-api: gpio: consumer: Mark another line of code as such
On Wed, Mar 3, 2021 at 9:43 PM Jonathan Neuschäfer wrote: > > Make it so that this #include line is rendered in monospace, like other > code blocks. > > Signed-off-by: Jonathan Neuschäfer > --- > Documentation/driver-api/gpio/consumer.rst | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/driver-api/gpio/consumer.rst > b/Documentation/driver-api/gpio/consumer.rst > index 22271c342d923..3366a991b4aa7 100644 > --- a/Documentation/driver-api/gpio/consumer.rst > +++ b/Documentation/driver-api/gpio/consumer.rst > @@ -12,7 +12,7 @@ Guidelines for GPIOs consumers > > Drivers that can't work without standard GPIO calls should have Kconfig > entries > that depend on GPIOLIB or select GPIOLIB. The functions that allow a driver > to > -obtain and use GPIOs are available by including the following file: > +obtain and use GPIOs are available by including the following file:: > > #include > > -- > 2.30.1 > Patch applied, thanks! Bartosz
Re: [PATCH v2 09/12] gpio: sim: new testing module
On Mon, Mar 8, 2021 at 4:05 PM Andy Shevchenko wrote: > > On Mon, Mar 08, 2021 at 03:23:31PM +0100, Bartosz Golaszewski wrote: > > On Fri, Mar 5, 2021 at 11:15 AM Andy Shevchenko > > wrote: > > > On Thu, Mar 04, 2021 at 09:15:29PM +0100, Bartosz Golaszewski wrote: > > > > On Thu, Mar 4, 2021 at 2:15 PM Andy Shevchenko > > > > wrote: > > > > > On Thu, Mar 04, 2021 at 11:24:49AM +0100, Bartosz Golaszewski wrote: > > > > > > From: Bartosz Golaszewski > > > > > > > + > > > > > > + /* > > > > > > + * FIXME If anyone knows a better way to parse that - please > > > > > > let me > > > > > > + * know. > > > > > > + */ > > > > > > > > > > If comma can be replaced with ' ' (space) then why not to use > > > > > next_arg() from > > > > > cmdline.c? I.o.w. do you have strong opinion why should we use comma > > > > > here? > > > > > > > > > > > > > My opinion is not very strong but I wanted to make the list of names > > > > resemble what we pass to the gpio-line-names property in device tree. > > > > Doesn't next_arg() react differently to string of the form: "foo=bar"? > > > > > > It's ambiguous here. > > > > > > So, the strings '"foo=bar"' and 'foo=bar' (w/o single quotes!) are indeed > > > parsed differently, i.e. > > > '"foo=bar"' -> 'foo=bar', > > > while > > > "foo=bar" -> 'foo' + 'bar'. > > > > > > > IMO '"foo", "bar", "", "foobar"' looks better than '"foo" "bar" "" > > "foobar"' and I'm also not sure next_arg will understand an empty > > quote? > > I guess it understands it. But I agree that comma-separated it would look > better. > > > If you're not objecting strongly, then I would prefer my version. > > I have strong opinion not to open code "yet another parser". > > So, grepping on 'strsep(.*, ",")' shows a lot of code that wants something > like > this. Interesting are the net/9p cases. This in particular pointed out to > lib/parser.c which in turn shows promising match_strlcpy() / match_strdup(). I > haven't looked deeply though. > > That said, I agree that next_arg() is not the best here. > > -- > With Best Regards, > Andy Shevchenko > > Shall we revisit this once it's upstream with a generalization for separating comma separated strings? Bart
Re: [PATCH 1/3] mfd: lp87565: fix typo in define names
On Fri, Feb 19, 2021 at 11:39 PM Luca Ceresoli wrote: > > "GOIO" should be "GPIO" here. > > Signed-off-by: Luca Ceresoli > --- For GPIO part: Acked-by: Bartosz Golaszewski
Re: [PATCH v2 09/12] gpio: sim: new testing module
On Fri, Mar 5, 2021 at 11:15 AM Andy Shevchenko wrote: > > On Thu, Mar 04, 2021 at 09:15:29PM +0100, Bartosz Golaszewski wrote: > > On Thu, Mar 4, 2021 at 2:15 PM Andy Shevchenko > > wrote: > > > On Thu, Mar 04, 2021 at 11:24:49AM +0100, Bartosz Golaszewski wrote: > > > > From: Bartosz Golaszewski > > > > > + > > > > + /* > > > > + * FIXME If anyone knows a better way to parse that - please let > > > > me > > > > + * know. > > > > + */ > > > > > > If comma can be replaced with ' ' (space) then why not to use next_arg() > > > from > > > cmdline.c? I.o.w. do you have strong opinion why should we use comma here? > > > > > > > My opinion is not very strong but I wanted to make the list of names > > resemble what we pass to the gpio-line-names property in device tree. > > Doesn't next_arg() react differently to string of the form: "foo=bar"? > > It's ambiguous here. > > So, the strings '"foo=bar"' and 'foo=bar' (w/o single quotes!) are indeed > parsed differently, i.e. > '"foo=bar"' -> 'foo=bar', > while > "foo=bar" -> 'foo' + 'bar'. > IMO '"foo", "bar", "", "foobar"' looks better than '"foo" "bar" "" "foobar"' and I'm also not sure next_arg will understand an empty quote? If you're not objecting strongly, then I would prefer my version. > ... > > > > > + ida_free(_sim_ida, id); > > > > > > Isn't it atomic per se? I mean that IDA won't give the same ID until you > > > free > > > it. I.o.w. why is it under the mutex? > > > > > > > You're right but if we rapidly create and destroy chips we'll be left > > with holes in the numbering (because new devices would be created > > before the IDA numbers are freed, so the driver would take a larger > > number that's currently free). It doesn't hurt but it would look worse > > IMO. Do you have a strong opinion on this? > > It's not strong per se, but I would rather follow the 2nd rule of locking: > don't protect something which doesn't need it. > OK, makes sense. > -- > With Best Regards, > Andy Shevchenko > >
Re: [PATCH v1 1/1] gpiolib: Read "gpio-line-names" from a firmware node
On Mon, Mar 8, 2021 at 12:45 PM Andy Shevchenko wrote: > > On Sun, Mar 07, 2021 at 06:14:49PM +0200, Andy Shevchenko wrote: > > On Sun, Mar 7, 2021 at 4:22 PM Bartosz Golaszewski > > wrote: > > > On Fri, Mar 5, 2021 at 1:02 PM Andy Shevchenko > > > wrote: > > > > > > > > On STM32MP1, the GPIO banks are subnodes of pin-controller@50002000, > > > > see arch/arm/boot/dts/stm32mp151.dtsi. The driver for > > > > pin-controller@50002000 is in drivers/pinctrl/stm32/pinctrl-stm32.c > > > > and iterates over all of its DT subnodes when registering each GPIO > > > > bank gpiochip. Each gpiochip has: > > > > > > > > - gpio_chip.parent = dev, > > > > where dev is the device node of the pin controller > > > > - gpio_chip.of_node = np, > > > > which is the OF node of the GPIO bank > > > > > > > > Therefore, dev_fwnode(chip->parent) != of_fwnode_handle(chip.of_node), > > > > i.e. pin-controller@50002000 != pin-controller@50002000/gpio@5000*000. > > > > > > > > The original code behaved correctly, as it extracted the > > > > "gpio-line-names" > > > > from of_fwnode_handle(chip.of_node) = > > > > pin-controller@50002000/gpio@5000*000. > > > > > > > > To achieve the same behaviour, read property from the firmware node. > > ... > > > > Did you run the OF unit tests on this? The check for the parent dev > > > was added after a bug was reported that was only triggered in unit > > > tests. > > > > Parent is not used anymore. But I can run unittests next week (or if > > you know that they are failing now, can you please show the failure?). > > For the record: > [ 40.587868] ### dt-test ### end of unittest - 190 passed, 0 failed > > If you have tests failed, we need more information about what line fails, etc. > > -- > With Best Regards, > Andy Shevchenko > > No it's fine, I just wanted to make sure. Patch applied, thanks! Bartosz
