[PATCH -next] gpio: mockup: remove gpio debugfs when remove device

Wei Yongjun posted 1 patch 3 years, 7 months ago
There is a newer version of this series
drivers/gpio/gpio-mockup.c | 11 +++++++++++
1 file changed, 11 insertions(+)
[PATCH -next] gpio: mockup: remove gpio debugfs when remove device
Posted by Wei Yongjun 3 years, 7 months ago
GPIO mockup debugfs is created in gpio_mockup_probe() but
forgot to remove when remove device. This patch add a devm
managed callback for removing them.

Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
 drivers/gpio/gpio-mockup.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c
index 8943cea92764..1fdc444b093b 100644
--- a/drivers/gpio/gpio-mockup.c
+++ b/drivers/gpio/gpio-mockup.c
@@ -373,6 +373,13 @@ static void gpio_mockup_debugfs_setup(struct device *dev,
 	}
 }
 
+static void gpio_mockup_debugfs_cleanup(void *data)
+{
+	struct gpio_mockup_chip *chip = data;
+
+	debugfs_remove_recursive(chip->dbg_dir);
+}
+
 static void gpio_mockup_dispose_mappings(void *data)
 {
 	struct gpio_mockup_chip *chip = data;
@@ -455,6 +462,10 @@ static int gpio_mockup_probe(struct platform_device *pdev)
 
 	gpio_mockup_debugfs_setup(dev, chip);
 
+	rv = devm_add_action_or_reset(dev, gpio_mockup_debugfs_cleanup, chip);
+	if (rv)
+		return rv;
+
 	return 0;
 }
 
-- 
2.34.1
Re: [PATCH -next] gpio: mockup: remove gpio debugfs when remove device
Posted by Bartosz Golaszewski 3 years, 7 months ago
On Tue, Aug 16, 2022 at 4:34 PM Wei Yongjun <weiyongjun1@huawei.com> wrote:
>
> GPIO mockup debugfs is created in gpio_mockup_probe() but
> forgot to remove when remove device. This patch add a devm
> managed callback for removing them.
>

The tag -next is for patches that address issues that are in next but
not yet in master.

> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> ---
>  drivers/gpio/gpio-mockup.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c
> index 8943cea92764..1fdc444b093b 100644
> --- a/drivers/gpio/gpio-mockup.c
> +++ b/drivers/gpio/gpio-mockup.c
> @@ -373,6 +373,13 @@ static void gpio_mockup_debugfs_setup(struct device *dev,
>         }
>  }
>
> +static void gpio_mockup_debugfs_cleanup(void *data)
> +{
> +       struct gpio_mockup_chip *chip = data;
> +
> +       debugfs_remove_recursive(chip->dbg_dir);
> +}
> +
>  static void gpio_mockup_dispose_mappings(void *data)
>  {
>         struct gpio_mockup_chip *chip = data;
> @@ -455,6 +462,10 @@ static int gpio_mockup_probe(struct platform_device *pdev)
>
>         gpio_mockup_debugfs_setup(dev, chip);
>
> +       rv = devm_add_action_or_reset(dev, gpio_mockup_debugfs_cleanup, chip);
> +       if (rv)
> +               return rv;
> +

Please return that function directly.

>         return 0;
>  }
>
> --
> 2.34.1
>

This isn't very relevant as the module needs to be unloaded anyway in
order to reconfigure the simulated device but I'll apply it as it's
technically correct. Did you see we have a new one - gpio-sim - that
uses configfs?

Bart
Re: [PATCH -next] gpio: mockup: remove gpio debugfs when remove device
Posted by weiyongjun (A) 3 years, 7 months ago
Hi Bart,

On 2022/8/19 20:49, Bartosz Golaszewski wrote:
> On Tue, Aug 16, 2022 at 4:34 PM Wei Yongjun <weiyongjun1@huawei.com> wrote:
>>
>> GPIO mockup debugfs is created in gpio_mockup_probe() but
>> forgot to remove when remove device. This patch add a devm
>> managed callback for removing them.
>>
> 
> The tag -next is for patches that address issues that are in next but
> not yet in master.
>>


> 
> This isn't very relevant as the module needs to be unloaded anyway in
> order to reconfigure the simulated device but I'll apply it as it's
> technically correct. Did you see we have a new one - gpio-sim - that
> uses configfs?
> 
> Bart
> 


I am using gpio-mockup as a interrupt-controller with the change[1],
it works will with overfs dts[2], and can success mockup device and 
trigger the irq. But when switch to gpio-sim, device can not be created 
by dts[3]. Not sure what's wrong with it. Any suggestion?


---------------[1]--------------------------
diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c
index a2e505a7545c..2b103861fa06 100644
--- a/drivers/gpio/gpio-mockup.c
+++ b/drivers/gpio/gpio-mockup.c
@@ -447,7 +451,7 @@ static int gpio_mockup_probe(struct platform_device 
*pdev)
  	for (i = 0; i < gc->ngpio; i++)
  		chip->lines[i].dir = GPIO_LINE_DIRECTION_IN;

-	chip->irq_sim_domain = devm_irq_domain_create_sim(dev, NULL,
+	chip->irq_sim_domain = devm_irq_domain_create_sim(dev, dev->fwnode,
  							  gc->ngpio);
  	if (IS_ERR(chip->irq_sim_domain))
  		return PTR_ERR(chip->irq_sim_domain);
diff --git a/kernel/irq/irq_sim.c b/kernel/irq/irq_sim.c
index dd76323ea3fd..e8f71f806a85 100644
--- a/kernel/irq/irq_sim.c
+++ b/kernel/irq/irq_sim.c
@@ -149,6 +153,7 @@ static void irq_sim_domain_unmap(struct irq_domain 
*domain, unsigned int virq)
  static const struct irq_domain_ops irq_sim_domain_ops = {
  	.map		= irq_sim_domain_map,
  	.unmap		= irq_sim_domain_unmap,
+	.xlate		= irq_domain_xlate_twocell,
  };

--------------------------------------------

---------------[2]--------------------------
/dts-v1/;
/plugin/;

#include <dt-bindings/interrupt-controller/irq.h>

&{/} {
	clk24m: clk24m {
		compatible = "fixed-clock";
		clock-output-names = "clk24m";
		clock-frequency = <24000000>;
		#clock-cells = <0>;
	};

	gpio: gpio {
		compatible = "gpio-mockup";

		gpio-base = <0>;
		nr-gpios = <0x200000>;

		gpio-controller;
		#gpio-cells = <2>;

		interrupt-controller;
		#interrupt-cells = <2>;

		line_b-hog {
			gpio-hog;
			gpios = <0 1>;
			input;
			line-name = "irq-sim";
		};
	};
};

&{/spi} {
	can0: can@1 {
		compatible = "microchip,mcp2515";
		reg = <1>;
		clocks = <&clk24m>;
		interrupts-extended = <&gpio 0 IRQ_TYPE_EDGE_BOTH>;
	};
};

-------------------------------------------


---------------[3]--------------------------

/dts-v1/;
/plugin/;

#include <dt-bindings/interrupt-controller/irq.h>

&{/} {
	clk24m: clk24m {
		compatible = "fixed-clock";
		clock-output-names = "clk24m";
		clock-frequency = <24000000>;
		#clock-cells = <0>;
	};

	gpio-sim {
		compatible = "gpio-simulator";

		bank0: bank0 {
			gpio-controller;
			#gpio-cells = <2>;
			ngpios = <16>;

			interrupt-controller;
			#interrupt-cells = <1>;
		};
	};
};

&{/spi} {
	can0: can@1 {
		compatible = "microchip,mcp2515";
		reg = <1>;
		clocks = <&clk24m>;
		interrupts-extended = <&bank0 0 IRQ_TYPE_EDGE_BOTH>;
	};
};

-------------------------------------------
Re: [PATCH -next] gpio: mockup: remove gpio debugfs when remove device
Posted by Bartosz Golaszewski 3 years, 7 months ago
On Mon, Aug 22, 2022 at 3:27 PM weiyongjun (A) <weiyongjun1@huawei.com> wrote:
>
> Hi Bart,
>
> On 2022/8/19 20:49, Bartosz Golaszewski wrote:
> > On Tue, Aug 16, 2022 at 4:34 PM Wei Yongjun <weiyongjun1@huawei.com> wrote:
> >>
> >> GPIO mockup debugfs is created in gpio_mockup_probe() but
> >> forgot to remove when remove device. This patch add a devm
> >> managed callback for removing them.
> >>
> >
> > The tag -next is for patches that address issues that are in next but
> > not yet in master.
> >>
>
>
> >
> > This isn't very relevant as the module needs to be unloaded anyway in
> > order to reconfigure the simulated device but I'll apply it as it's
> > technically correct. Did you see we have a new one - gpio-sim - that
> > uses configfs?
> >
> > Bart
> >
>
>
> I am using gpio-mockup as a interrupt-controller with the change[1],
> it works will with overfs dts[2], and can success mockup device and
> trigger the irq. But when switch to gpio-sim, device can not be created
> by dts[3]. Not sure what's wrong with it. Any suggestion?
>

I see you submitted a patch series for gpio-sim - does it fix this issue?

Bart
Re: [PATCH -next] gpio: mockup: remove gpio debugfs when remove device
Posted by weiyongjun (A) 3 years, 7 months ago
Hi Bart,

On 2022/8/28 22:00, Bartosz Golaszewski wrote:
> On Mon, Aug 22, 2022 at 3:27 PM weiyongjun (A) <weiyongjun1@huawei.com> wrote:
>>
>> Hi Bart,
>>
>> On 2022/8/19 20:49, Bartosz Golaszewski wrote:
>>> On Tue, Aug 16, 2022 at 4:34 PM Wei Yongjun <weiyongjun1@huawei.com> wrote:
>>>>
>>>> GPIO mockup debugfs is created in gpio_mockup_probe() but
>>>> forgot to remove when remove device. This patch add a devm
>>>> managed callback for removing them.
>>>>
>>>
>>> The tag -next is for patches that address issues that are in next but
>>> not yet in master.
>>>>
>>
>>
>>>
>>> This isn't very relevant as the module needs to be unloaded anyway in
>>> order to reconfigure the simulated device but I'll apply it as it's
>>> technically correct. Did you see we have a new one - gpio-sim - that
>>> uses configfs?
>>>
>>> Bart
>>>
>>
>>
>> I am using gpio-mockup as a interrupt-controller with the change[1],
>> it works will with overfs dts[2], and can success mockup device and
>> trigger the irq. But when switch to gpio-sim, device can not be created
>> by dts[3]. Not sure what's wrong with it. Any suggestion?
>>
> 
> I see you submitted a patch series for gpio-sim - does it fix this issue?


Yes, after that patch series, gpio-sim works well in my use case.

Thanks,
Wei Yongjun
Re: [PATCH -next] gpio: mockup: remove gpio debugfs when remove device
Posted by weiyongjun (A) 3 years, 7 months ago
On 2022/8/19 20:49, Bartosz Golaszewski wrote:
> On Tue, Aug 16, 2022 at 4:34 PM Wei Yongjun <weiyongjun1@huawei.com> wrote:
>> GPIO mockup debugfs is created in gpio_mockup_probe() but
>> forgot to remove when remove device. This patch add a devm
>> managed callback for removing them.
>>
> The tag -next is for patches that address issues that are in next but
> not yet in master.


Got it. Thanks.


>
>> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
>> ---
>>   drivers/gpio/gpio-mockup.c | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c
>> index 8943cea92764..1fdc444b093b 100644
>> --- a/drivers/gpio/gpio-mockup.c
>> +++ b/drivers/gpio/gpio-mockup.c
>> @@ -373,6 +373,13 @@ static void gpio_mockup_debugfs_setup(struct device *dev,
>>          }
>>   }
>>
>> +static void gpio_mockup_debugfs_cleanup(void *data)
>> +{
>> +       struct gpio_mockup_chip *chip = data;
>> +
>> +       debugfs_remove_recursive(chip->dbg_dir);
>> +}
>> +
>>   static void gpio_mockup_dispose_mappings(void *data)
>>   {
>>          struct gpio_mockup_chip *chip = data;
>> @@ -455,6 +462,10 @@ static int gpio_mockup_probe(struct platform_device *pdev)
>>
>>          gpio_mockup_debugfs_setup(dev, chip);
>>
>> +       rv = devm_add_action_or_reset(dev, gpio_mockup_debugfs_cleanup, chip);
>> +       if (rv)
>> +               return rv;
>> +
> Please return that function directly.


Will fix it.


>
>>          return 0;
>>   }
>>
>> --
>> 2.34.1
>>
> This isn't very relevant as the module needs to be unloaded anyway in
> order to reconfigure the simulated device but I'll apply it as it's
> technically correct. Did you see we have a new one - gpio-sim - that
> uses configfs?
>

gpio-mockup is used for testing other drivers with overlay dts, and 
triggered

this issue. I though gpio-sim not work with dts, but it should works well.

I will have a try to using the new gpio-sim module.