* [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()
2023-03-06 9:00 [PATCH v6 00/13] Add pinctrl sysfs and RZ/G2L POEG support Biju Das
@ 2023-03-06 9:00 ` Biju Das
2023-03-06 23:33 ` andy.shevchenko
2023-03-06 9:00 ` [PATCH v6 02/13] pinctrl: Add poutdisops variable to struct pinctrl_desc Biju Das
` (10 subsequent siblings)
11 siblings, 1 reply; 44+ messages in thread
From: Biju Das @ 2023-03-06 9:00 UTC (permalink / raw)
To: Linus Walleij
Cc: Biju Das, linux-gpio, Geert Uytterhoeven, Thierry Reding,
Uwe Kleine-König, Prabhakar Mahadev Lad, linux-renesas-soc
Add pinctrl_get_device() to find a device handle associated with
a pincontrol group(i.e. by matching function name and group name for a
device). This device handle can then be used for finding match for the
pin output disable device that protects device against short circuits
on the pins.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v6:
* New patch
Ref:
https://lore.kernel.org/linux-renesas-soc/OS0PR01MB5922F5494D3C0862E15F3F8486B39@OS0PR01MB5922.jpnprd01.prod.outlook.com/T/#t
---
drivers/pinctrl/core.c | 49 ++++++++++++++++++++++++++++++++
include/linux/pinctrl/consumer.h | 9 ++++++
2 files changed, 58 insertions(+)
diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index d6e6c751255f..2ba222026db4 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -27,6 +27,7 @@
#include <linux/pinctrl/devinfo.h>
#include <linux/pinctrl/machine.h>
#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
#ifdef CONFIG_GPIOLIB
#include "../gpio/gpiolib.h"
@@ -1584,6 +1585,54 @@ int pinctrl_select_default_state(struct device *dev)
}
EXPORT_SYMBOL_GPL(pinctrl_select_default_state);
+static bool pinctrl_get_device_match(struct pinctrl_setting *setting,
+ const char *fname, const char *gname)
+{
+ struct pinctrl_dev *pctldev = setting->pctldev;
+ const struct pinmux_ops *pmxops = pctldev->desc->pmxops;
+ const struct pinctrl_ops *pctlops = pctldev->desc->pctlops;
+ const char *function = pmxops->get_function_name(pctldev,
+ setting->data.mux.func);
+ const char *group = pctlops->get_group_name(pctldev,
+ setting->data.mux.group);
+
+ if ((!strcmp(function, fname)) && (!strcmp(group, gname)))
+ return true;
+
+ return false;
+}
+
+/**
+ * pinctrl_get_device() - returns device associated with a pincontrol group
+ * @fname: function name
+ * @gname: group name
+ */
+struct device *pinctrl_get_device(const char *fname, const char *gname)
+{
+ struct pinctrl *p;
+ struct pinctrl_state *state;
+ struct pinctrl_setting *setting;
+
+ mutex_lock(&pinctrl_list_mutex);
+
+ list_for_each_entry(p, &pinctrl_list, node) {
+ list_for_each_entry(state, &p->states, node) {
+ list_for_each_entry(setting, &state->settings, node) {
+ if (setting->type == PIN_MAP_TYPE_MUX_GROUP &&
+ pinctrl_get_device_match(setting, fname, gname)) {
+ mutex_unlock(&pinctrl_list_mutex);
+ return p->dev;
+ }
+ }
+ }
+ }
+
+ mutex_unlock(&pinctrl_list_mutex);
+
+ return ERR_PTR(-EINVAL);
+}
+EXPORT_SYMBOL_GPL(pinctrl_get_device);
+
#ifdef CONFIG_PM
/**
diff --git a/include/linux/pinctrl/consumer.h b/include/linux/pinctrl/consumer.h
index 4729d54e8995..6ff8857c0a9c 100644
--- a/include/linux/pinctrl/consumer.h
+++ b/include/linux/pinctrl/consumer.h
@@ -42,6 +42,9 @@ extern struct pinctrl * __must_check devm_pinctrl_get(struct device *dev);
extern void devm_pinctrl_put(struct pinctrl *p);
extern int pinctrl_select_default_state(struct device *dev);
+extern struct device * __must_check pinctrl_get_device(const char *fname,
+ const char *gname);
+
#ifdef CONFIG_PM
extern int pinctrl_pm_select_default_state(struct device *dev);
extern int pinctrl_pm_select_sleep_state(struct device *dev);
@@ -142,6 +145,12 @@ static inline int pinctrl_pm_select_idle_state(struct device *dev)
return 0;
}
+static inline struct device * __must_check pinctrl_get_device(const char *fname,
+ const char *gname)
+{
+ return NULL;
+}
+
#endif /* CONFIG_PINCTRL */
static inline struct pinctrl * __must_check pinctrl_get_select(struct device *dev,
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()
2023-03-06 9:00 ` [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device() Biju Das
@ 2023-03-06 23:33 ` andy.shevchenko
2023-03-07 8:13 ` Biju Das
0 siblings, 1 reply; 44+ messages in thread
From: andy.shevchenko @ 2023-03-06 23:33 UTC (permalink / raw)
To: Biju Das
Cc: Linus Walleij, linux-gpio, Geert Uytterhoeven, Thierry Reding,
Uwe Kleine-König, Prabhakar Mahadev Lad, linux-renesas-soc
Mon, Mar 06, 2023 at 09:00:02AM +0000, Biju Das kirjoitti:
> Add pinctrl_get_device() to find a device handle associated with
> a pincontrol group(i.e. by matching function name and group name for a
> device). This device handle can then be used for finding match for the
> pin output disable device that protects device against short circuits
> on the pins.
Not sure I understand the use case. Please, create a better commit message.
Also it is missing the explanation why there will be no collisions when looking
by the same pair of function and group name from different device.
(Always imagine that you have 2+ same IP blocks on the platform before doing
any pin control core work. This will help you to design it properly. )
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()
2023-03-06 23:33 ` andy.shevchenko
@ 2023-03-07 8:13 ` Biju Das
2023-03-09 12:54 ` Andy Shevchenko
2023-03-09 13:05 ` Linus Walleij
0 siblings, 2 replies; 44+ messages in thread
From: Biju Das @ 2023-03-07 8:13 UTC (permalink / raw)
To: andy.shevchenko@gmail.com
Cc: Linus Walleij, linux-gpio@vger.kernel.org, Geert Uytterhoeven,
Thierry Reding, Uwe Kleine-König, Prabhakar Mahadev Lad,
linux-renesas-soc@vger.kernel.org
Hi Andy Shevchenko,
Thanks for the feedback.
> Subject: Re: [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()
>
> Mon, Mar 06, 2023 at 09:00:02AM +0000, Biju Das kirjoitti:
> > Add pinctrl_get_device() to find a device handle associated with a
> > pincontrol group(i.e. by matching function name and group name for a
> > device). This device handle can then be used for finding match for the
> > pin output disable device that protects device against short circuits
> > on the pins.
>
> Not sure I understand the use case. Please, create a better commit message.
OK, Basically pinmux_enable_setting allows exclusive access of pin to a device.
It won't allow multiple devices to claim a pin.
Maybe instead of find device handle, it should re worded as to find the
current owner of the pins associated with a given function and group name??
Or please let me know have better suggestion.
Then use this info for matching the device used for pin output
disable.
>
> Also it is missing the explanation why there will be no collisions when
> looking by the same pair of function and group name from different device.
setting->data.mux will be unique for a pin. So there won't be a collision when
looking by the same pair of function and group name from different device.
>
> (Always imagine that you have 2+ same IP blocks on the platform before doing
> any pin control core work. This will help you to design it properly. )
OK.
Cheers,
Biju
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()
2023-03-07 8:13 ` Biju Das
@ 2023-03-09 12:54 ` Andy Shevchenko
2023-03-09 13:26 ` Biju Das
2023-03-09 13:05 ` Linus Walleij
1 sibling, 1 reply; 44+ messages in thread
From: Andy Shevchenko @ 2023-03-09 12:54 UTC (permalink / raw)
To: Biju Das
Cc: Linus Walleij, linux-gpio@vger.kernel.org, Geert Uytterhoeven,
Thierry Reding, Uwe Kleine-König, Prabhakar Mahadev Lad,
linux-renesas-soc@vger.kernel.org
On Tue, Mar 7, 2023 at 10:13 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > Subject: Re: [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()
> > Mon, Mar 06, 2023 at 09:00:02AM +0000, Biju Das kirjoitti:
...
> > > Add pinctrl_get_device() to find a device handle associated with a
> > > pincontrol group(i.e. by matching function name and group name for a
> > > device). This device handle can then be used for finding match for the
> > > pin output disable device that protects device against short circuits
> > > on the pins.
> >
> > Not sure I understand the use case. Please, create a better commit message.
>
> OK, Basically pinmux_enable_setting allows exclusive access of pin to a device.
> It won't allow multiple devices to claim a pin.
Which is correct. No? Show me the schematics of the real use case for that.
The owner of the pin is the host side. I can't imagine how the same
pin is shared inside the SoC. Is it broken hardware design?
...
> > Also it is missing the explanation why there will be no collisions when
> > looking by the same pair of function and group name from different device.
>
> setting->data.mux will be unique for a pin. So there won't be a collision when
> looking by the same pair of function and group name from different device.
>
> > (Always imagine that you have 2+ same IP blocks on the platform before doing
> > any pin control core work. This will help you to design it properly. )
Not sure how the pair function_name group_name makes the device unique.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()
2023-03-09 12:54 ` Andy Shevchenko
@ 2023-03-09 13:26 ` Biju Das
2023-03-09 13:44 ` Andy Shevchenko
0 siblings, 1 reply; 44+ messages in thread
From: Biju Das @ 2023-03-09 13:26 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Linus Walleij, linux-gpio@vger.kernel.org, Geert Uytterhoeven,
Thierry Reding, Uwe Kleine-König, Prabhakar Mahadev Lad,
linux-renesas-soc@vger.kernel.org
Hi Andy,
Thanks for the feedback.
> Subject: Re: [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()
>
> On Tue, Mar 7, 2023 at 10:13 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > Subject: Re: [PATCH v6 01/13] pinctrl: core: Add
> > > pinctrl_get_device() Mon, Mar 06, 2023 at 09:00:02AM +0000, Biju Das
> kirjoitti:
>
> ...
>
> > > > Add pinctrl_get_device() to find a device handle associated with a
> > > > pincontrol group(i.e. by matching function name and group name for
> > > > a device). This device handle can then be used for finding match
> > > > for the pin output disable device that protects device against
> > > > short circuits on the pins.
> > >
> > > Not sure I understand the use case. Please, create a better commit
> message.
> >
> > OK, Basically pinmux_enable_setting allows exclusive access of pin to a
> device.
> > It won't allow multiple devices to claim a pin.
>
> Which is correct. No? Show me the schematics of the real use case for that.
>
> The owner of the pin is the host side. I can't imagine how the same pin is
> shared inside the SoC. Is it broken hardware design?
We are discussing usage of
echo "fname gname" and you asked a question whether multiple devices can
claim a pin at the same time
and my answer is no.
as setting->data.mux will be unique for a pin and will be claimed by
device during commit state.
Am I missing anything here??
Cheers,
Biju
>
> ...
>
> > > Also it is missing the explanation why there will be no collisions
> > > when looking by the same pair of function and group name from different
> device.
> >
> > setting->data.mux will be unique for a pin. So there won't be a
> > setting->collision when
> > looking by the same pair of function and group name from different device.
> >
> > > (Always imagine that you have 2+ same IP blocks on the platform
> > > before doing any pin control core work. This will help you to design
> > > it properly. )
>
> Not sure how the pair function_name group_name makes the device unique.
Do you agree Device handle + function_name + group_name make it unique.
For pin outdisable we are making use of this 3 combination.
See the details.
This patch series adds support for controlling output disable function using
sysfs in a generic way as described below.
| A | | B | | C | | D | | E |
|user space|--->|pinctrl core|<-->|SoC pinctrl|<-->|Output disable|--|PWM|
| | | | | | | | | |
A executes command to configure a pin group for pin output disable operation
echo "fname gname conf conf_val" > configure
B parses the command and identifies the binding device associated with that
pin group
C matches the binding device against the device registered by D for
configuration operation
D matches the pin group and configure the pins for Output disable operation
Both D and E are linked together by dt(i.e. PWM channel linked with Output
disable Port)
Cheers,
Biju
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()
2023-03-09 13:26 ` Biju Das
@ 2023-03-09 13:44 ` Andy Shevchenko
2023-03-09 14:19 ` Biju Das
0 siblings, 1 reply; 44+ messages in thread
From: Andy Shevchenko @ 2023-03-09 13:44 UTC (permalink / raw)
To: Biju Das
Cc: Linus Walleij, linux-gpio@vger.kernel.org, Geert Uytterhoeven,
Thierry Reding, Uwe Kleine-König, Prabhakar Mahadev Lad,
linux-renesas-soc@vger.kernel.org
On Thu, Mar 9, 2023 at 3:26 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > Subject: Re: [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()
> > On Tue, Mar 7, 2023 at 10:13 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > > Subject: Re: [PATCH v6 01/13] pinctrl: core: Add
> > > > pinctrl_get_device() Mon, Mar 06, 2023 at 09:00:02AM +0000, Biju Das
> > kirjoitti:
...
> > > > > Add pinctrl_get_device() to find a device handle associated with a
> > > > > pincontrol group(i.e. by matching function name and group name for
> > > > > a device). This device handle can then be used for finding match
> > > > > for the pin output disable device that protects device against
> > > > > short circuits on the pins.
> > > >
> > > > Not sure I understand the use case. Please, create a better commit
> > message.
> > >
> > > OK, Basically pinmux_enable_setting allows exclusive access of pin to a
> > device.
> > > It won't allow multiple devices to claim a pin.
This is a confusion you brought. You got us completely lost. Please,
try again from the clean sheet.
> > Which is correct. No? Show me the schematics of the real use case for that.
> >
> > The owner of the pin is the host side. I can't imagine how the same pin is
> > shared inside the SoC. Is it broken hardware design?
>
> We are discussing usage of
>
> echo "fname gname" and you asked a question whether multiple devices can
> claim a pin at the same time
>
> and my answer is no.
> as setting->data.mux will be unique for a pin and will be claimed by
> device during commit state.
>
> Am I missing anything here??
Yes. The same fname/gname can be in many *pin control* (provider) devices.
So, it does not uniquely define the pin control device.
...
> > > > Also it is missing the explanation why there will be no collisions
> > > > when looking by the same pair of function and group name from different
> > device.
> > >
> > > setting->data.mux will be unique for a pin. So there won't be a
> > > setting->collision when
> > > looking by the same pair of function and group name from different device.
> > >
> > > > (Always imagine that you have 2+ same IP blocks on the platform
> > > > before doing any pin control core work. This will help you to design
> > > > it properly. )
> >
> > Not sure how the pair function_name group_name makes the device unique.
>
> Do you agree Device handle + function_name + group_name make it unique.
Yes.
> For pin outdisable we are making use of this 3 combination.
> See the details.
>
>
> This patch series adds support for controlling output disable function using
> sysfs in a generic way as described below.
>
> | A | | B | | C | | D | | E |
> |user space|--->|pinctrl core|<-->|SoC pinctrl|<-->|Output disable|--|PWM|
> | | | | | | | | | |
>
> A executes command to configure a pin group for pin output disable operation
> echo "fname gname conf conf_val" > configure
>
> B parses the command and identifies the binding device associated with that
> pin group
>
> C matches the binding device against the device registered by D for
> configuration operation
>
> D matches the pin group and configure the pins for Output disable operation
>
> Both D and E are linked together by dt(i.e. PWM channel linked with Output
> disable Port)
Sounds like an overengineered hack to achieve something that I can't
read between the lines. Why is this so complicated interface and flow
are needed to begin with?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()
2023-03-09 13:44 ` Andy Shevchenko
@ 2023-03-09 14:19 ` Biju Das
2023-03-13 20:42 ` Biju Das
2023-03-13 22:05 ` Linus Walleij
0 siblings, 2 replies; 44+ messages in thread
From: Biju Das @ 2023-03-09 14:19 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Linus Walleij, linux-gpio@vger.kernel.org, Geert Uytterhoeven,
Thierry Reding, Uwe Kleine-König, Prabhakar Mahadev Lad,
linux-renesas-soc@vger.kernel.org
Hi Andy,
Thanks for the explanation.
> Subject: Re: [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()
>
> On Thu, Mar 9, 2023 at 3:26 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > Subject: Re: [PATCH v6 01/13] pinctrl: core: Add
> > > pinctrl_get_device() On Tue, Mar 7, 2023 at 10:13 AM Biju Das
> <biju.das.jz@bp.renesas.com> wrote:
> > > > > Subject: Re: [PATCH v6 01/13] pinctrl: core: Add
> > > > > pinctrl_get_device() Mon, Mar 06, 2023 at 09:00:02AM +0000, Biju
> > > > > Das
> > > kirjoitti:
>
> ...
>
> > > > > > Add pinctrl_get_device() to find a device handle associated
> > > > > > with a pincontrol group(i.e. by matching function name and
> > > > > > group name for a device). This device handle can then be used
> > > > > > for finding match for the pin output disable device that
> > > > > > protects device against short circuits on the pins.
> > > > >
> > > > > Not sure I understand the use case. Please, create a better
> > > > > commit
> > > message.
> > > >
> > > > OK, Basically pinmux_enable_setting allows exclusive access of pin
> > > > to a
> > > device.
> > > > It won't allow multiple devices to claim a pin.
>
> This is a confusion you brought. You got us completely lost. Please, try
> again from the clean sheet.
>
> > > Which is correct. No? Show me the schematics of the real use case for
> that.
> > >
> > > The owner of the pin is the host side. I can't imagine how the same
> > > pin is shared inside the SoC. Is it broken hardware design?
> >
> > We are discussing usage of
> >
> > echo "fname gname" and you asked a question whether multiple devices
> > can claim a pin at the same time
> >
> > and my answer is no.
>
> > as setting->data.mux will be unique for a pin and will be claimed by
> > device during commit state.
> >
> > Am I missing anything here??
>
> Yes. The same fname/gname can be in many *pin control* (provider) devices.
I agree.
I have used the code used for [1] getting *pin control* devices associated with function name
and group name.
[1]
cat /sys/kernel/debug/pinctrl/pinctrl-handles
if output of [1], can return multiple devices for a given fname/gname, then I am wrong.
Please correct me if that is the case.
So I was under the impression that [2], it will fail if multiple devices try to acquire a pin.
[2]
https://elixir.bootlin.com/linux/latest/source/drivers/pinctrl/pinmux.c#L132
>
> So, it does not uniquely define the pin control device.
I agree.
>
> ...
>
> > > > > Also it is missing the explanation why there will be no
> > > > > collisions when looking by the same pair of function and group
> > > > > name from different
> > > device.
> > > >
> > > > setting->data.mux will be unique for a pin. So there won't be a
> > > > setting->collision when
> > > > looking by the same pair of function and group name from different
> device.
> > > >
> > > > > (Always imagine that you have 2+ same IP blocks on the platform
> > > > > before doing any pin control core work. This will help you to
> > > > > design it properly. )
> > >
> > > Not sure how the pair function_name group_name makes the device unique.
> >
> > Do you agree Device handle + function_name + group_name make it unique.
>
> Yes.
>
> > For pin outdisable we are making use of this 3 combination.
> > See the details.
> >
> >
> > This patch series adds support for controlling output disable function
> > using sysfs in a generic way as described below.
> >
> > | A | | B | | C | | D | | E |
> > |user space|--->|pinctrl core|<-->|SoC pinctrl|<-->|Output disable|--|PWM|
> > | | | | | | | | | |
> >
> > A executes command to configure a pin group for pin output disable
> operation
> > echo "fname gname conf conf_val" > configure
> >
> > B parses the command and identifies the binding device associated with
> that
> > pin group
> >
> > C matches the binding device against the device registered by D for
> > configuration operation
> >
> > D matches the pin group and configure the pins for Output disable
> > operation
> >
> > Both D and E are linked together by dt(i.e. PWM channel linked with
> > Output disable Port)
>
> Sounds like an overengineered hack to achieve something that I can't read
> between the lines. Why is this so complicated interface and flow are needed
> to begin with?
It is very simple.
I am trying to give details like how a pwm pins used for pin output disable
is configured from user space in a generic way.
My use case is,
I have an IP which detects short circuit between the output terminals and
disable the output from pwm pins ,when it detects short circuit to protect from
system failure.
pwm-pins are involved in this operation.
From user space we need to configure the type of protection for this pins (eg: disable PWM output,
when both pwm outputs are high at same time).
For that, we need to find a provider device (which provides gpt-pins).
pinctrl_get_device() returns "current provider device" associated with fname/gname.
If " current provider device" == "pwm device" do the configuration.
Cheers,
Biju
^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()
2023-03-09 14:19 ` Biju Das
@ 2023-03-13 20:42 ` Biju Das
2023-03-13 22:05 ` Linus Walleij
1 sibling, 0 replies; 44+ messages in thread
From: Biju Das @ 2023-03-13 20:42 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Linus Walleij, linux-gpio@vger.kernel.org, Geert Uytterhoeven,
Thierry Reding, Uwe Kleine-König, Prabhakar Mahadev Lad,
linux-renesas-soc@vger.kernel.org
Hi Andy,
> Subject: RE: [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()
>
> Hi Andy,
>
> Thanks for the explanation.
>
> > Subject: Re: [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()
> >
> > On Thu, Mar 9, 2023 at 3:26 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > > > Subject: Re: [PATCH v6 01/13] pinctrl: core: Add
> > > > pinctrl_get_device() On Tue, Mar 7, 2023 at 10:13 AM Biju Das
> > <biju.das.jz@bp.renesas.com> wrote:
> > > > > > Subject: Re: [PATCH v6 01/13] pinctrl: core: Add
> > > > > > pinctrl_get_device() Mon, Mar 06, 2023 at 09:00:02AM +0000,
> > > > > > Biju Das
> > > > kirjoitti:
> >
> > ...
> >
> > > > > > > Add pinctrl_get_device() to find a device handle associated
> > > > > > > with a pincontrol group(i.e. by matching function name and
> > > > > > > group name for a device). This device handle can then be
> > > > > > > used for finding match for the pin output disable device
> > > > > > > that protects device against short circuits on the pins.
> > > > > >
> > > > > > Not sure I understand the use case. Please, create a better
> > > > > > commit
> > > > message.
> > > > >
> > > > > OK, Basically pinmux_enable_setting allows exclusive access of
> > > > > pin to a
> > > > device.
> > > > > It won't allow multiple devices to claim a pin.
> >
> > This is a confusion you brought. You got us completely lost. Please,
> > try again from the clean sheet.
> >
> > > > Which is correct. No? Show me the schematics of the real use case
> > > > for
> > that.
> > > >
> > > > The owner of the pin is the host side. I can't imagine how the
> > > > same pin is shared inside the SoC. Is it broken hardware design?
> > >
> > > We are discussing usage of
> > >
> > > echo "fname gname" and you asked a question whether multiple devices
> > > can claim a pin at the same time
> > >
> > > and my answer is no.
> >
> > > as setting->data.mux will be unique for a pin and will be claimed by
> > > device during commit state.
> > >
> > > Am I missing anything here??
> >
> > Yes. The same fname/gname can be in many *pin control* (provider) devices.
>
> I agree.
>
> I have used the code used for [1] getting *pin control* devices associated
> with function name and group name.
>
> [1]
> cat /sys/kernel/debug/pinctrl/pinctrl-handles
>
> if output of [1], can return multiple devices for a given fname/gname, then
> I am wrong.
> Please correct me if that is the case.
>
> So I was under the impression that [2], it will fail if multiple devices try
> to acquire a pin.
>
> [2]
> https://elixir.bootlin.com/linux/latest/source/drivers/pinctrl/pinmux.c#L132
I have done the below modifications in my dt and confirm that, pinctrl framework
doesn't allow 2 devices to claim pins with a given function name and group name
at same time.
dt-changes:
&gpt {
+ gpt6-pins {
+ pinmux = <RZG2L_PORT_PINMUX(44, 0, 5)>, /* GTIOC6A */
+ <RZG2L_PORT_PINMUX(44, 1, 5)>; /* GTIOC6B */
+ };
+
+ gpt7-pins {
+ pinmux = <RZG2L_PORT_PINMUX(44, 2, 5)>, /* GTIOC7A */
+ <RZG2L_PORT_PINMUX(44, 3, 5)>; /* GTIOC7B */
+ };
};
&mtu {
+ gpt6-pins {
+ pinmux = <RZG2L_PORT_PINMUX(44, 0, 4)>, /* MTIOC3A */
+ <RZG2L_PORT_PINMUX(44, 1, 4)>, /* MTIOC3B */
+ <RZG2L_PORT_PINMUX(44, 2, 4)>, /* MTIOC3C */
+ <RZG2L_PORT_PINMUX(44, 3, 4)>; /* MTIOC3D */
};
};
Initially MTU device is claiming the pins P44_0 and P44_1.
root@smarc-rzg2l:~# cat /sys/kernel/debug/pinctrl/pinctrl-handles
device: 10001200.timer current state: default
state: default
type: MUX_GROUP controller pinctrl-rzg2l group: mtu3_zphase_clk (3) function: mtu3_zphase_clk (3)
type: MUX_GROUP controller pinctrl-rzg2l group: mtu3_clk (4) function: mtu3_clk (4)
type: MUX_GROUP controller pinctrl-rzg2l group: gpt6-pins (5) function: gpt6-pins (5)
When It tried to load gpt, I get the below erro,
[ 15.024714] pinctrl-rzg2l 11030000.pinctrl: pin P44_0 already requested by 10001200.timer; cannot claim for 10048000.pwm
Starting Update UTMP about System Runlevel Changes...
[ 15.044828] pinctrl-rzg2l 11030000.pinctrl: pin-352 (10048000.pwm) status -22
[ 15.056515] pinctrl-rzg2l 11030000.pinctrl: could not request pin 352 (P44_0) from group gpt6-pins on device pinctrl-rzg2l
[ 15.070224] pwm-rzg2l-gpt 10048000.pwm: Error applying setting, reverse things back
After unloading mtu modules and loading gpt modules, gpt module is claims the pin.
root@smarc-rzg2l:~# rmmod rz_mtu3_cnt
root@smarc-rzg2l:~# rmmod pwm_rz_mtu3
root@smarc-rzg2l:~# cd /sys/bus/platform/drivers/rz-mtu3/
root@smarc-rzg2l:/sys/bus/platform/drivers/rz-mtu3# echo 10001200.timer > unbind
root@smarc-rzg2l:/sys/bus/platform/drivers/rz-mtu3# rmmod rzg2l_poeg
root@smarc-rzg2l:/sys/bus/platform/drivers/rz-mtu3# rmmod pwm_rzg2l_gpt
root@smarc-rzg2l:/sys/bus/platform/drivers/rz-mtu3# cat /sys/kernel/debug/pinctrl/pinctrl-handles | grep gpt
root@smarc-rzg2l:/sys/bus/platform/drivers/rz-mtu3#
root@smarc-rzg2l:/sys/bus/platform/drivers/rz-mtu3# modprobe rzg2l_poeg
root@smarc-rzg2l:/sys/bus/platform/drivers/rz-mtu3# modprobe pwm_rzg2l_gpt
root@smarc-rzg2l:/sys/bus/platform/drivers/rz-mtu3# cat /sys/kernel/debug/pinctrl/pinctrl-handles | grep gpt
type: MUX_GROUP controller pinctrl-rzg2l group: gpt6-pins (5) function: gpt6-pins (5)
type: MUX_GROUP controller pinctrl-rzg2l group: gpt7-pins (11) function: gpt7-pins (11)
root@smarc-rzg2l:/sys/bus/platform/drivers/rz-mtu3# cat /sys/kernel/debug/pinctrl/pinctrl-handles
device: 10048000.pwm current state: default
state: default
type: MUX_GROUP controller pinctrl-rzg2l group: gpt6-pins (5) function: gpt6-pins (5)
type: MUX_GROUP controller pinctrl-rzg2l group: gpt7-pins (11) function: gpt7-pins (11)
Since I have used same function name/ group name it gets a unique selector(5) and the functionality for
gpt is not working as the selector meant for MTU.
But if I replace "gpt6-pins" -> "mtu-pins" in MTU node, then the functionality works as expected as
the selector is different for mtu-pins and gpt-pins eventhough both uses same pins.
So I believe current implementation is based on /sys/kernel/debug/pinctrl/pinctrl-handles is
good unless I miss something here. Please correct me,if you think my observation is wrong.
Cheers,
Biju
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()
2023-03-09 14:19 ` Biju Das
2023-03-13 20:42 ` Biju Das
@ 2023-03-13 22:05 ` Linus Walleij
2023-03-14 8:27 ` Biju Das
1 sibling, 1 reply; 44+ messages in thread
From: Linus Walleij @ 2023-03-13 22:05 UTC (permalink / raw)
To: Biju Das
Cc: Andy Shevchenko, linux-gpio@vger.kernel.org, Geert Uytterhoeven,
Thierry Reding, Uwe Kleine-König, Prabhakar Mahadev Lad,
linux-renesas-soc@vger.kernel.org
On Thu, Mar 9, 2023 at 3:19 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> I have an IP which detects short circuit between the output terminals and
> disable the output from pwm pins ,when it detects short circuit to protect from
> system failure.
>
> pwm-pins are involved in this operation.
>
> From user space we need to configure the type of protection for this pins (eg: disable PWM output,
> when both pwm outputs are high at same time).
Why do you want to do this from user space?
It sounds like something the kernel should be doing.
The kernel has a PWM subsystem, and a pin control subsystem,
and we don't even have a userspace ABI for pin control.
Pin control is designed to avoid electrical disasters
and a driver can add further policy for sure.
If you want to add policy of different types to avoid electrical
disaster into the pin control driver, go ahead, just run it by
Geert so he's on board with the ideas.
> For that, we need to find a provider device (which provides gpt-pins).
>
> pinctrl_get_device() returns "current provider device" associated with fname/gname.
> If " current provider device" == "pwm device" do the configuration.
I don't understand this, sorry.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()
2023-03-13 22:05 ` Linus Walleij
@ 2023-03-14 8:27 ` Biju Das
2023-03-14 8:42 ` Linus Walleij
2023-03-14 9:33 ` Geert Uytterhoeven
0 siblings, 2 replies; 44+ messages in thread
From: Biju Das @ 2023-03-14 8:27 UTC (permalink / raw)
To: Linus Walleij
Cc: Andy Shevchenko, linux-gpio@vger.kernel.org, Geert Uytterhoeven,
Thierry Reding, Uwe Kleine-König, Prabhakar Mahadev Lad,
linux-renesas-soc@vger.kernel.org, Rob Herring,
Krzysztof Kozlowski
Hi Linus W,
Thanks for feedback.
> Subject: Re: [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()
>
> On Thu, Mar 9, 2023 at 3:19 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
>
> > I have an IP which detects short circuit between the output terminals
> > and disable the output from pwm pins ,when it detects short circuit to
> > protect from system failure.
> >
> > pwm-pins are involved in this operation.
> >
> > From user space we need to configure the type of protection for this
> > pins (eg: disable PWM output, when both pwm outputs are high at same
> time).
>
> Why do you want to do this from user space?
To take care of the below features provided by Port Output Enable for GPT(a.k.a PWM)
(POEG) IP.
The output pins of the general PWM timer (GPT) can be disabled by
using the port output enabling function for the GPT (POEG).
Specifically, either of the following ways can be used[1].
[1]
Use case 1)
● Input level detection of the GTETRGA to GTETRGD pins (i.e: detect short circuit in switching circuit
externally and use an external pin(GTETRGA to GTETRGD) to disable the output pins of PWM)
Use case 2)
● Output-disable request from the GPT (GPT detects short circuit in switching circuit internally and
disable the output pins of PWM)
Use case 3)
● Register settings(Detect short circuit in switching circuit
externally and use internal register to disable the output pins of PWM)
The advantage of providing these options from user space is, it is flexible.
Runtime user can configure the use case he wants to use for his product.
+Rob, + Krzysztof Kozlowski
If we cannot do it in user space, then we need to make it as part of
Dt bindings and users will define the use case they need in DT.
Some of the failure conditions in switching circuits are like below
when you use PWM push-pull configuration you SHOULD have a dead time.
When + mosfet is turned off - mosfet can't be immediately turned on
because you will create a direct path (short circuit) between + and -
as parasitic capacitance will leave + mosfet turned on for a while .
This will destroy your mosfets.
Dead time is the delay measured from turning off the driver switch
connected to one rail of the power supply to the time the switch
connected to the other rail of the power supply is turned on.
Switching devices like MOSFETs and IGBTs turn off after a delay
when the gate drive is turned off. If the other switch on the half
bridge is turned on immediately, both upper and lower switches may be
in a conductive region for a brief moment, shorting the power rail.
In order to avoid this, a dead time is maintained between turning off
of one switch and turning on the other in a half bridge.
POEG IP provides the below protections,
1) When the GTIOCA pin and the GTIOCB pin(PWM pins) are driven to an active level simultaneously, the
PWM generates an output-disable request to the POEG. Through reception of these requests,
the POEG can control whether the GTIOCA and GTIOCB pins are output-disabled
2) PWM output pins can be set to be disabled when the PWM output pins detect a dead time error
or short circuit detection between the output terminals
>
> It sounds like something the kernel should be doing.
If we cannot do the above use cases[1] in user space, then we need to make it as part of
Dt bindings and it will be fully taken care in kernel.
>
> The kernel has a PWM subsystem, and a pin control subsystem, and we don't
> even have a userspace ABI for pin control.
> Pin control is designed to avoid electrical disasters and a driver can add
> further policy for sure.
>
> If you want to add policy of different types to avoid electrical disaster
> into the pin control driver, go ahead, just run it by Geert so he's on board
> with the ideas.
OK. Geert Can you please provide valuable suggestion how to address this use cases[1]
As mentioned above?
Currently Pin control driver is used for identifying the pwm device by using
pwm gname/fname and configuring the various use cases as mentioned above[1]
for a system.
>
> > For that, we need to find a provider device (which provides gpt-pins).
> >
> > pinctrl_get_device() returns "current provider device" associated with
> fname/gname.
> > If " current provider device" == "pwm device" do the configuration.
>
> I don't understand this, sorry.
Andy mentioned, the same fname/gname can be used in many *pin control* (provider) devices.
But when checking the code(/sys/kernel/debug/pinctrl/pinctrl-handles), at a given time,
there will be only one device claims Pins associated with a given fname/gname.
Currently pinctrl_get_device() returns the current *pin control* (provider) device
that claimed pins associated with a given fname/gname.
Cheers,
Biju
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()
2023-03-14 8:27 ` Biju Das
@ 2023-03-14 8:42 ` Linus Walleij
2023-03-28 7:08 ` Biju Das
2023-03-14 9:33 ` Geert Uytterhoeven
1 sibling, 1 reply; 44+ messages in thread
From: Linus Walleij @ 2023-03-14 8:42 UTC (permalink / raw)
To: Biju Das
Cc: Andy Shevchenko, linux-gpio@vger.kernel.org, Geert Uytterhoeven,
Thierry Reding, Uwe Kleine-König, Prabhakar Mahadev Lad,
linux-renesas-soc@vger.kernel.org, Rob Herring,
Krzysztof Kozlowski
On Tue, Mar 14, 2023 at 9:27 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> If we cannot do it in user space, then we need to make it as part of
> Dt bindings and users will define the use case they need in DT.
That sounds like a much better idea :)
The kernel is for protecting hardware from users after all, and it
seems you want to select one of these use cases and DT is
excellent for system config like this.
So I would say work ahead on this path.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()
2023-03-14 8:42 ` Linus Walleij
@ 2023-03-28 7:08 ` Biju Das
0 siblings, 0 replies; 44+ messages in thread
From: Biju Das @ 2023-03-28 7:08 UTC (permalink / raw)
To: Linus Walleij
Cc: Andy Shevchenko, linux-gpio@vger.kernel.org, Geert Uytterhoeven,
Thierry Reding, Uwe Kleine-König, Prabhakar Mahadev Lad,
linux-renesas-soc@vger.kernel.org, Rob Herring,
Krzysztof Kozlowski
Hi Linus Walleij,
Thanks for the feedback.
> <krzysztof.kozlowski+dt@linaro.org>
> Subject: Re: [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()
>
> On Tue, Mar 14, 2023 at 9:27 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
>
> > If we cannot do it in user space, then we need to make it as part of
> > Dt bindings and users will define the use case they need in DT.
>
> That sounds like a much better idea :)
>
> The kernel is for protecting hardware from users after all, and it seems you
> want to select one of these use cases and DT is excellent for system config
> like this.
>
> So I would say work ahead on this path.
Agreed. Will send next version based on this approach.
Cheers,
Biju
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()
2023-03-14 8:27 ` Biju Das
2023-03-14 8:42 ` Linus Walleij
@ 2023-03-14 9:33 ` Geert Uytterhoeven
2023-03-14 11:33 ` Biju Das
1 sibling, 1 reply; 44+ messages in thread
From: Geert Uytterhoeven @ 2023-03-14 9:33 UTC (permalink / raw)
To: Biju Das
Cc: Linus Walleij, Andy Shevchenko, linux-gpio@vger.kernel.org,
Thierry Reding, Uwe Kleine-König, Prabhakar Mahadev Lad,
linux-renesas-soc@vger.kernel.org, Rob Herring,
Krzysztof Kozlowski
Hi Biju,
On Tue, Mar 14, 2023 at 9:27 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > Subject: Re: [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()
> > On Thu, Mar 9, 2023 at 3:19 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > I have an IP which detects short circuit between the output terminals
> > > and disable the output from pwm pins ,when it detects short circuit to
> > > protect from system failure.
> > >
> > > pwm-pins are involved in this operation.
> > >
> > > From user space we need to configure the type of protection for this
> > > pins (eg: disable PWM output, when both pwm outputs are high at same
> > time).
> >
> > Why do you want to do this from user space?
>
> To take care of the below features provided by Port Output Enable for GPT(a.k.a PWM)
> (POEG) IP.
>
> The output pins of the general PWM timer (GPT) can be disabled by
> using the port output enabling function for the GPT (POEG).
> Specifically, either of the following ways can be used[1].
>
> [1]
>
> Use case 1)
> ● Input level detection of the GTETRGA to GTETRGD pins (i.e: detect short circuit in switching circuit
> externally and use an external pin(GTETRGA to GTETRGD) to disable the output pins of PWM)
>
> Use case 2)
> ● Output-disable request from the GPT (GPT detects short circuit in switching circuit internally and
> disable the output pins of PWM)
>
> Use case 3)
> ● Register settings(Detect short circuit in switching circuit
> externally and use internal register to disable the output pins of PWM)
>
> The advantage of providing these options from user space is, it is flexible.
> Runtime user can configure the use case he wants to use for his product.
>
> +Rob, + Krzysztof Kozlowski
>
> If we cannot do it in user space, then we need to make it as part of
> Dt bindings and users will define the use case they need in DT.
>
> Some of the failure conditions in switching circuits are like below
>
> when you use PWM push-pull configuration you SHOULD have a dead time.
> When + mosfet is turned off - mosfet can't be immediately turned on
> because you will create a direct path (short circuit) between + and -
> as parasitic capacitance will leave + mosfet turned on for a while .
> This will destroy your mosfets.
>
> Dead time is the delay measured from turning off the driver switch
> connected to one rail of the power supply to the time the switch
> connected to the other rail of the power supply is turned on.
> Switching devices like MOSFETs and IGBTs turn off after a delay
> when the gate drive is turned off. If the other switch on the half
> bridge is turned on immediately, both upper and lower switches may be
> in a conductive region for a brief moment, shorting the power rail.
> In order to avoid this, a dead time is maintained between turning off
> of one switch and turning on the other in a half bridge.
>
> POEG IP provides the below protections,
>
> 1) When the GTIOCA pin and the GTIOCB pin(PWM pins) are driven to an active level simultaneously, the
> PWM generates an output-disable request to the POEG. Through reception of these requests,
> the POEG can control whether the GTIOCA and GTIOCB pins are output-disabled
>
> 2) PWM output pins can be set to be disabled when the PWM output pins detect a dead time error
> or short circuit detection between the output terminals
>
> >
> > It sounds like something the kernel should be doing.
>
> If we cannot do the above use cases[1] in user space, then we need to make it as part of
> Dt bindings and it will be fully taken care in kernel.
>
> >
> > The kernel has a PWM subsystem, and a pin control subsystem, and we don't
> > even have a userspace ABI for pin control.
> > Pin control is designed to avoid electrical disasters and a driver can add
> > further policy for sure.
> >
> > If you want to add policy of different types to avoid electrical disaster
> > into the pin control driver, go ahead, just run it by Geert so he's on board
> > with the ideas.
>
> OK. Geert Can you please provide valuable suggestion how to address this use cases[1]
> As mentioned above?
Is this configuration you need to do once, based on the hardware,
or do you need to change it at run-time?
Before, I was under the impression you needed to change the
configuration at run-time, hence the need for a sysfs API.
If configuration is static, DT is the way to go.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 44+ messages in thread* RE: [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()
2023-03-14 9:33 ` Geert Uytterhoeven
@ 2023-03-14 11:33 ` Biju Das
2023-03-15 8:00 ` Geert Uytterhoeven
0 siblings, 1 reply; 44+ messages in thread
From: Biju Das @ 2023-03-14 11:33 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Linus Walleij, Andy Shevchenko, linux-gpio@vger.kernel.org,
Thierry Reding, Uwe Kleine-König, Prabhakar Mahadev Lad,
linux-renesas-soc@vger.kernel.org, Rob Herring,
Krzysztof Kozlowski
Hi Geert,
Thanks for the feedback.
> Subject: Re: [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()
>
> Hi Biju,
>
> On Tue, Mar 14, 2023 at 9:27 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > Subject: Re: [PATCH v6 01/13] pinctrl: core: Add
> > > pinctrl_get_device() On Thu, Mar 9, 2023 at 3:19 PM Biju Das
> <biju.das.jz@bp.renesas.com> wrote:
> > > > I have an IP which detects short circuit between the output
> > > > terminals and disable the output from pwm pins ,when it detects
> > > > short circuit to protect from system failure.
> > > >
> > > > pwm-pins are involved in this operation.
> > > >
> > > > From user space we need to configure the type of protection for
> > > > this pins (eg: disable PWM output, when both pwm outputs are high
> > > > at same
> > > time).
> > >
> > > Why do you want to do this from user space?
> >
> > To take care of the below features provided by Port Output Enable for
> > GPT(a.k.a PWM)
> > (POEG) IP.
> >
> > The output pins of the general PWM timer (GPT) can be disabled by
> > using the port output enabling function for the GPT (POEG).
> > Specifically, either of the following ways can be used[1].
> >
> > [1]
> >
> > Use case 1)
> > ● Input level detection of the GTETRGA to GTETRGD pins (i.e: detect
> > short circuit in switching circuit externally and use an external
> > pin(GTETRGA to GTETRGD) to disable the output pins of PWM)
> >
> > Use case 2)
> > ● Output-disable request from the GPT (GPT detects short circuit in
> > switching circuit internally and disable the output pins of PWM)
> >
> > Use case 3)
> > ● Register settings(Detect short circuit in switching circuit
> > externally and use internal register to disable the output pins of
> > PWM)
> >
> > The advantage of providing these options from user space is, it is
> flexible.
> > Runtime user can configure the use case he wants to use for his product.
> >
> > +Rob, + Krzysztof Kozlowski
> >
> > If we cannot do it in user space, then we need to make it as part of
> > Dt bindings and users will define the use case they need in DT.
> >
> > Some of the failure conditions in switching circuits are like below
> >
> > when you use PWM push-pull configuration you SHOULD have a dead time.
> > When + mosfet is turned off - mosfet can't be immediately turned on
> > because you will create a direct path (short circuit) between + and -
> > as parasitic capacitance will leave + mosfet turned on for a while .
> > This will destroy your mosfets.
> >
> > Dead time is the delay measured from turning off the driver switch
> > connected to one rail of the power supply to the time the switch
> > connected to the other rail of the power supply is turned on.
> > Switching devices like MOSFETs and IGBTs turn off after a delay when
> > the gate drive is turned off. If the other switch on the half bridge
> > is turned on immediately, both upper and lower switches may be in a
> > conductive region for a brief moment, shorting the power rail.
> > In order to avoid this, a dead time is maintained between turning off
> > of one switch and turning on the other in a half bridge.
> >
> > POEG IP provides the below protections,
> >
> > 1) When the GTIOCA pin and the GTIOCB pin(PWM pins) are driven to an
> > active level simultaneously, the PWM generates an output-disable
> > request to the POEG. Through reception of these requests, the POEG can
> > control whether the GTIOCA and GTIOCB pins are output-disabled
> >
> > 2) PWM output pins can be set to be disabled when the PWM output pins
> > detect a dead time error or short circuit detection between the output
> > terminals
> >
> > >
> > > It sounds like something the kernel should be doing.
> >
> > If we cannot do the above use cases[1] in user space, then we need to
> > make it as part of Dt bindings and it will be fully taken care in kernel.
> >
> > >
> > > The kernel has a PWM subsystem, and a pin control subsystem, and we
> > > don't even have a userspace ABI for pin control.
> > > Pin control is designed to avoid electrical disasters and a driver
> > > can add further policy for sure.
> > >
> > > If you want to add policy of different types to avoid electrical
> > > disaster into the pin control driver, go ahead, just run it by Geert
> > > so he's on board with the ideas.
> >
> > OK. Geert Can you please provide valuable suggestion how to address
> > this use cases[1] As mentioned above?
>
> Is this configuration you need to do once, based on the hardware, or do you
> need to change it at run-time?
I think this configuration needed only once based on the hardware.
>
> Before, I was under the impression you needed to change the configuration at
> run-time, hence the need for a sysfs API.
> If configuration is static, DT is the way to go.
At that time, I was not explored the possibility of creating poeg char device.
For eg: After the initial setting in DT, I guess with poeg char device we should be able to
achieve below use cases.
Use case 1)
We can provide user space event indicating, Output-disable request from the GTETRGn pin occurred.
and provide some options (rd/wr file ops) to user space to clear the fault.
Use case 2)
We can provide user space event indicating, Output-disable request from GPT disable request occurred.
and provide some options(rd/wr file ops) to user space to clear the fault.
Use case 3)
User space to control Output-disable through rd/wr file ops.
Please let me know is it ok or am I missing something here??
Cheers,
Biju
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()
2023-03-14 11:33 ` Biju Das
@ 2023-03-15 8:00 ` Geert Uytterhoeven
0 siblings, 0 replies; 44+ messages in thread
From: Geert Uytterhoeven @ 2023-03-15 8:00 UTC (permalink / raw)
To: Biju Das
Cc: Linus Walleij, Andy Shevchenko, linux-gpio@vger.kernel.org,
Thierry Reding, Uwe Kleine-König, Prabhakar Mahadev Lad,
linux-renesas-soc@vger.kernel.org, Rob Herring,
Krzysztof Kozlowski
Hi Biju,
On Tue, Mar 14, 2023 at 12:33 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > Subject: Re: [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()
> > On Tue, Mar 14, 2023 at 9:27 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > > Subject: Re: [PATCH v6 01/13] pinctrl: core: Add
> > > > pinctrl_get_device() On Thu, Mar 9, 2023 at 3:19 PM Biju Das
> > <biju.das.jz@bp.renesas.com> wrote:
> > > > > I have an IP which detects short circuit between the output
> > > > > terminals and disable the output from pwm pins ,when it detects
> > > > > short circuit to protect from system failure.
> > > > >
> > > > > pwm-pins are involved in this operation.
> > > > >
> > > > > From user space we need to configure the type of protection for
> > > > > this pins (eg: disable PWM output, when both pwm outputs are high
> > > > > at same
> > > > time).
> > > >
> > > > Why do you want to do this from user space?
> > >
> > > To take care of the below features provided by Port Output Enable for
> > > GPT(a.k.a PWM)
> > > (POEG) IP.
> > >
> > > The output pins of the general PWM timer (GPT) can be disabled by
> > > using the port output enabling function for the GPT (POEG).
> > > Specifically, either of the following ways can be used[1].
> > >
> > > [1]
> > >
> > > Use case 1)
> > > ● Input level detection of the GTETRGA to GTETRGD pins (i.e: detect
> > > short circuit in switching circuit externally and use an external
> > > pin(GTETRGA to GTETRGD) to disable the output pins of PWM)
> > >
> > > Use case 2)
> > > ● Output-disable request from the GPT (GPT detects short circuit in
> > > switching circuit internally and disable the output pins of PWM)
> > >
> > > Use case 3)
> > > ● Register settings(Detect short circuit in switching circuit
> > > externally and use internal register to disable the output pins of
> > > PWM)
> > >
> > > The advantage of providing these options from user space is, it is
> > flexible.
> > > Runtime user can configure the use case he wants to use for his product.
> > >
> > > +Rob, + Krzysztof Kozlowski
> > >
> > > If we cannot do it in user space, then we need to make it as part of
> > > Dt bindings and users will define the use case they need in DT.
> > >
> > > Some of the failure conditions in switching circuits are like below
> > >
> > > when you use PWM push-pull configuration you SHOULD have a dead time.
> > > When + mosfet is turned off - mosfet can't be immediately turned on
> > > because you will create a direct path (short circuit) between + and -
> > > as parasitic capacitance will leave + mosfet turned on for a while .
> > > This will destroy your mosfets.
> > >
> > > Dead time is the delay measured from turning off the driver switch
> > > connected to one rail of the power supply to the time the switch
> > > connected to the other rail of the power supply is turned on.
> > > Switching devices like MOSFETs and IGBTs turn off after a delay when
> > > the gate drive is turned off. If the other switch on the half bridge
> > > is turned on immediately, both upper and lower switches may be in a
> > > conductive region for a brief moment, shorting the power rail.
> > > In order to avoid this, a dead time is maintained between turning off
> > > of one switch and turning on the other in a half bridge.
> > >
> > > POEG IP provides the below protections,
> > >
> > > 1) When the GTIOCA pin and the GTIOCB pin(PWM pins) are driven to an
> > > active level simultaneously, the PWM generates an output-disable
> > > request to the POEG. Through reception of these requests, the POEG can
> > > control whether the GTIOCA and GTIOCB pins are output-disabled
> > >
> > > 2) PWM output pins can be set to be disabled when the PWM output pins
> > > detect a dead time error or short circuit detection between the output
> > > terminals
> > >
> > > >
> > > > It sounds like something the kernel should be doing.
> > >
> > > If we cannot do the above use cases[1] in user space, then we need to
> > > make it as part of Dt bindings and it will be fully taken care in kernel.
> > >
> > > >
> > > > The kernel has a PWM subsystem, and a pin control subsystem, and we
> > > > don't even have a userspace ABI for pin control.
> > > > Pin control is designed to avoid electrical disasters and a driver
> > > > can add further policy for sure.
> > > >
> > > > If you want to add policy of different types to avoid electrical
> > > > disaster into the pin control driver, go ahead, just run it by Geert
> > > > so he's on board with the ideas.
> > >
> > > OK. Geert Can you please provide valuable suggestion how to address
> > > this use cases[1] As mentioned above?
> >
> > Is this configuration you need to do once, based on the hardware, or do you
> > need to change it at run-time?
>
> I think this configuration needed only once based on the hardware.
OK, so using DT for that would be fine.
> > Before, I was under the impression you needed to change the configuration at
> > run-time, hence the need for a sysfs API.
> > If configuration is static, DT is the way to go.
>
> At that time, I was not explored the possibility of creating poeg char device.
>
> For eg: After the initial setting in DT, I guess with poeg char device we should be able to
> achieve below use cases.
>
> Use case 1)
> We can provide user space event indicating, Output-disable request from the GTETRGn pin occurred.
> and provide some options (rd/wr file ops) to user space to clear the fault.
>
> Use case 2)
> We can provide user space event indicating, Output-disable request from GPT disable request occurred.
> and provide some options(rd/wr file ops) to user space to clear the fault.
>
> Use case 3)
> User space to control Output-disable through rd/wr file ops.
>
> Please let me know is it ok or am I missing something here??
Using a char device for that sounds fine to me.
If you just needed to clear the fault, you could use a device property in sysfs.
But that would still leave us without a way to provide events to userspace.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()
2023-03-07 8:13 ` Biju Das
2023-03-09 12:54 ` Andy Shevchenko
@ 2023-03-09 13:05 ` Linus Walleij
2023-03-09 13:18 ` Biju Das
1 sibling, 1 reply; 44+ messages in thread
From: Linus Walleij @ 2023-03-09 13:05 UTC (permalink / raw)
To: Biju Das
Cc: andy.shevchenko@gmail.com, linux-gpio@vger.kernel.org,
Geert Uytterhoeven, Thierry Reding, Uwe Kleine-König,
Prabhakar Mahadev Lad, linux-renesas-soc@vger.kernel.org
On Tue, Mar 7, 2023 at 9:13 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > Mon, Mar 06, 2023 at 09:00:02AM +0000, Biju Das kirjoitti:
> > > Add pinctrl_get_device() to find a device handle associated with a
> > > pincontrol group(i.e. by matching function name and group name for a
> > > device). This device handle can then be used for finding match for the
> > > pin output disable device that protects device against short circuits
> > > on the pins.
> >
> > Not sure I understand the use case. Please, create a better commit message.
>
> OK, Basically pinmux_enable_setting allows exclusive access of pin to a device.
> It won't allow multiple devices to claim a pin.
So what is the use case? Which two devices need to use the same
pin at the same time?
You can already:
1) Use the same pin with different devices at different times, because
pin configs can be changed arbitrarily at runtime, see for example:
drivers/i2c/muxes/i2c-demux-pinctrl.c
2) Mux a pin to a certain device *and* use it for GPIO at the same time,
all that is needed is to set .strict to false in struct pinmux_ops.
This should be false if e.g. the GPIO can be used to "sample" the
output of a I2C block connected to the same pins, so the two
functions (I2C and GPIO) are not electrically decoupled.
So do you really have a use case where two devices need to use the same
pin at the same time? I've seen much but I haven't seen this before!
Which two devices are that?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()
2023-03-09 13:05 ` Linus Walleij
@ 2023-03-09 13:18 ` Biju Das
0 siblings, 0 replies; 44+ messages in thread
From: Biju Das @ 2023-03-09 13:18 UTC (permalink / raw)
To: Linus Walleij
Cc: andy.shevchenko@gmail.com, linux-gpio@vger.kernel.org,
Geert Uytterhoeven, Thierry Reding, Uwe Kleine-König,
Prabhakar Mahadev Lad, linux-renesas-soc@vger.kernel.org
Hi Linus and Andy,
> Subject: Re: [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()
>
> On Tue, Mar 7, 2023 at 9:13 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > Mon, Mar 06, 2023 at 09:00:02AM +0000, Biju Das kirjoitti:
>
> > > > Add pinctrl_get_device() to find a device handle associated with a
> > > > pincontrol group(i.e. by matching function name and group name for
> > > > a device). This device handle can then be used for finding match
> > > > for the pin output disable device that protects device against
> > > > short circuits on the pins.
> > >
> > > Not sure I understand the use case. Please, create a better commit
> message.
> >
> > OK, Basically pinmux_enable_setting allows exclusive access of pin to a
> device.
> > It won't allow multiple devices to claim a pin.
>
> So what is the use case? Which two devices need to use the same pin at the
> same time?
Andy asked a question Can a pin be used by multiple devices at same time
My answer is no.
The reason is,
There will be a single owner claiming a pin at given time.
setting->data.mux will be unique for a pin. So there won't be a collision when
looking by the same pair of function and group name from different device.
>
> You can already:
>
> 1) Use the same pin with different devices at different times, because
> pin configs can be changed arbitrarily at runtime, see for example:
> drivers/i2c/muxes/i2c-demux-pinctrl.c
Agreed.
>
> 2) Mux a pin to a certain device *and* use it for GPIO at the same time,
> all that is needed is to set .strict to false in struct pinmux_ops.
> This should be false if e.g. the GPIO can be used to "sample" the
> output of a I2C block connected to the same pins, so the two
> functions (I2C and GPIO) are not electrically decoupled.
>
> So do you really have a use case where two devices need to use the same pin
> at the same time? I've seen much but I haven't seen this before!
> Which two devices are that?
I don't have a use case like that.
Cheers,
Biju
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v6 02/13] pinctrl: Add poutdisops variable to struct pinctrl_desc
2023-03-06 9:00 [PATCH v6 00/13] Add pinctrl sysfs and RZ/G2L POEG support Biju Das
2023-03-06 9:00 ` [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device() Biju Das
@ 2023-03-06 9:00 ` Biju Das
2023-03-06 9:00 ` [PATCH v6 03/13] pinctrl: Add sysfs support Biju Das
` (9 subsequent siblings)
11 siblings, 0 replies; 44+ messages in thread
From: Biju Das @ 2023-03-06 9:00 UTC (permalink / raw)
To: Linus Walleij
Cc: Biju Das, linux-gpio, Geert Uytterhoeven, Thierry Reding,
Uwe Kleine-König, Prabhakar Mahadev Lad, linux-renesas-soc
Add pin output disable operations vtable to struct pinctrl_desc.
This is needed for configuring IPs that support pin output disable
function.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v6:
* New patch
Ref:
https://lore.kernel.org/linux-renesas-soc/OS0PR01MB5922F5494D3C0862E15F3F8486B39@OS0PR01MB5922.jpnprd01.prod.outlook.com/T/#t
---
include/linux/pinctrl/output-disable.h | 42 ++++++++++++++++++++++++++
include/linux/pinctrl/pinctrl.h | 4 +++
2 files changed, 46 insertions(+)
create mode 100644 include/linux/pinctrl/output-disable.h
diff --git a/include/linux/pinctrl/output-disable.h b/include/linux/pinctrl/output-disable.h
new file mode 100644
index 000000000000..7f63dcc93e44
--- /dev/null
+++ b/include/linux/pinctrl/output-disable.h
@@ -0,0 +1,42 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Interface the output disable portions of the pinctrl subsystem
+ *
+ * Copyright (C) 2023 Renesas Electronics Corporation
+ * This interface is used in the core to keep track of output_disable pins.
+ *
+ * Author: Biju Das <biju.das.jz@bp.renesas.com>
+ */
+#ifndef __LINUX_PINCTRL_OUTPUT_DISABLE_H
+#define __LINUX_PINCTRL_OUTPUT_DISABLE_H
+
+#include <linux/types.h>
+#include <linux/pinctrl/consumer.h>
+
+struct pinctrl_dev;
+struct seq_file;
+
+enum pin_output_disable_conf {
+ PINCTRL_OUTPUT_DISABLE_BY_USER,
+ PINCTRL_OUTPUT_DISABLE_BY_SOC_ON_PIN_OUTPUT_HIGH,
+ PINCTRL_OUTPUT_DISABLE_BY_SOC_ON_PIN_OUTPUT_LOW,
+ PINCTRL_OUTPUT_DISABLE_BY_SOC_ON_DEAD_TIME_ERROR,
+ PINCTRL_OUTPUT_DISABLE_BY_LEVEL_DETECTION,
+};
+
+/**
+ * struct pin_config_set - pin output disable config operations, to be
+ * implemented by pin configuration capable drivers.
+ * @pin_output_disable_config_set: for pin controllers that want to use the
+ * generic interface, this flag tells the framework that it's generic.
+ */
+struct pin_output_disable_ops {
+ int (*pin_output_disable_config_set)(struct pinctrl_dev *pctldev,
+ struct device *dev,
+ const char *fname,
+ const char *gname,
+ enum pin_output_disable_conf conf,
+ unsigned int conf_val);
+};
+
+#endif /* __LINUX_PINCTRL_OUTPUT_DISABLE_H */
diff --git a/include/linux/pinctrl/pinctrl.h b/include/linux/pinctrl/pinctrl.h
index 4d252ea00ed1..3b5beee49039 100644
--- a/include/linux/pinctrl/pinctrl.h
+++ b/include/linux/pinctrl/pinctrl.h
@@ -25,6 +25,7 @@ struct pinconf_ops;
struct pinctrl_dev;
struct pinctrl_map;
struct pinmux_ops;
+struct pin_output_disable_ops;
/**
* struct pingroup - provides information on pingroup
@@ -135,6 +136,8 @@ struct pinctrl_ops {
* @pmxops: pinmux operations vtable, if you support pinmuxing in your driver
* @confops: pin config operations vtable, if you support pin configuration in
* your driver
+ * @poutdisops: pin output disable operations vtable, if you support pin output
+ * disable in your driver
* @owner: module providing the pin controller, used for refcounting
* @num_custom_params: Number of driver-specific custom parameters to be parsed
* from the hardware description
@@ -154,6 +157,7 @@ struct pinctrl_desc {
const struct pinctrl_ops *pctlops;
const struct pinmux_ops *pmxops;
const struct pinconf_ops *confops;
+ const struct pin_output_disable_ops *poutdisops;
struct module *owner;
#ifdef CONFIG_GENERIC_PINCONF
unsigned int num_custom_params;
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread* [PATCH v6 03/13] pinctrl: Add sysfs support
2023-03-06 9:00 [PATCH v6 00/13] Add pinctrl sysfs and RZ/G2L POEG support Biju Das
2023-03-06 9:00 ` [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device() Biju Das
2023-03-06 9:00 ` [PATCH v6 02/13] pinctrl: Add poutdisops variable to struct pinctrl_desc Biju Das
@ 2023-03-06 9:00 ` Biju Das
2023-03-06 23:38 ` andy.shevchenko
2023-03-07 13:41 ` Linus Walleij
2023-03-06 9:00 ` [PATCH v6 04/13] pinctrl: renesas: rzg2l: Add pin output disable support Biju Das
` (8 subsequent siblings)
11 siblings, 2 replies; 44+ messages in thread
From: Biju Das @ 2023-03-06 9:00 UTC (permalink / raw)
To: Linus Walleij
Cc: Biju Das, linux-gpio, Geert Uytterhoeven, Thierry Reding,
Uwe Kleine-König, Prabhakar Mahadev Lad, linux-renesas-soc
Add a simple sysfs interface to the generic pinctrl framework
for configuring pins for output disable operation.
/sys/class/pinctrl/
`-- output-disable/
|-- configure (w/o) ask the kernel to configure a pin group
for output disable operation.
echo "<group-name function-name 0 1>" > configure
The existing "pinmux-functions" debugfs file lists the pin functions
registered for the pin controller. For example:
function 0: usb0, groups = [ usb0 ]
function 1: usb1, groups = [ usb1 ]
function 2: gpt4-pins, groups = [ gpt4-pins ]
function 3: scif0, groups = [ scif0 ]
function 4: scif2, groups = [ scif2 ]
function 5: spi1, groups = [ spi1 ]
To configure gpt4-pins for output disable activation by user:
echo "gpt4-pins gpt4-pins 0 1" > configure
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v6:
* New patch
Ref:
https://lore.kernel.org/linux-renesas-soc/OS0PR01MB5922F5494D3C0862E15F3F8486B39@OS0PR01MB5922.jpnprd01.prod.outlook.com/T/#t
---
Documentation/ABI/testing/sysfs-class-pinctrl | 32 ++++
drivers/pinctrl/Kconfig | 4 +
drivers/pinctrl/Makefile | 1 +
drivers/pinctrl/core.c | 3 +
drivers/pinctrl/output-disable.c | 148 ++++++++++++++++++
drivers/pinctrl/output-disable.h | 32 ++++
6 files changed, 220 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-class-pinctrl
create mode 100644 drivers/pinctrl/output-disable.c
create mode 100644 drivers/pinctrl/output-disable.h
diff --git a/Documentation/ABI/testing/sysfs-class-pinctrl b/Documentation/ABI/testing/sysfs-class-pinctrl
new file mode 100644
index 000000000000..cdf47ea77c32
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-pinctrl
@@ -0,0 +1,32 @@
+What: /sys/class/pinctrl
+Date: March 2023
+KernelVersion: 6.4
+Contact: Biju Das <biju.das.jz@bp.renesas.com>
+Description:
+ The pinctrl class sub-directory belongs to the Generic pincontrol
+ Framework and provides a sysfs interface for using pincontrol
+ configurations.
+
+What: /sys/class/pinctrl/output-disable
+Date: March 2023
+KernelVersion: 6.4
+Contact: Biju Das <biju.das.jz@bp.renesas.com>
+Description:
+ A /sys/class/pinctrl/output-disable directory is created for
+ pin output disable function.
+
+What: /sys/class/pinctrl/output-disable/configure
+Date: March 2023
+KernelVersion: 6.4
+Contact: Biju Das <biju.das.jz@bp.renesas.com>
+Description:
+ The configuration for pin output disable IP.
+
+ Write the following string to configure the pin output disable:
+
+ - "fname gname conf conf_val"
+
+ fname is function name,
+ gname is group name,
+ conf is configuration
+ conf_val is 0 or 1, to disable/enable the above conf.
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index dcb53c4a9584..3553d76c3577 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -21,6 +21,10 @@ config GENERIC_PINMUX_FUNCTIONS
config PINCONF
bool "Support pin configuration controllers" if COMPILE_TEST
+config PIN_OUTPUT_DISABLE
+ bool
+ default y if SYSFS
+
config GENERIC_PINCONF
bool
select PINCONF
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index d5939840bb2a..e923adaa2b9c 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -6,6 +6,7 @@ subdir-ccflags-$(CONFIG_DEBUG_PINCTRL) += -DDEBUG
obj-y += core.o pinctrl-utils.o
obj-$(CONFIG_PINMUX) += pinmux.o
obj-$(CONFIG_PINCONF) += pinconf.o
+obj-$(CONFIG_PIN_OUTPUT_DISABLE) += output-disable.o
obj-$(CONFIG_GENERIC_PINCONF) += pinconf-generic.o
obj-$(CONFIG_OF) += devicetree.o
diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index 2ba222026db4..3fce50786754 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -36,6 +36,7 @@
#include "core.h"
#include "devicetree.h"
+#include "output-disable.h"
#include "pinconf.h"
#include "pinmux.h"
@@ -2171,6 +2172,7 @@ int pinctrl_enable(struct pinctrl_dev *pctldev)
mutex_unlock(&pinctrldev_list_mutex);
pinctrl_init_device_debugfs(pctldev);
+ output_disable_init_device_sysfs(pctldev);
return 0;
}
@@ -2252,6 +2254,7 @@ void pinctrl_unregister(struct pinctrl_dev *pctldev)
mutex_lock(&pctldev->mutex);
pinctrl_remove_device_debugfs(pctldev);
+ output_disable_remove_device_sysfs(pctldev);
mutex_unlock(&pctldev->mutex);
if (!IS_ERR_OR_NULL(pctldev->p))
diff --git a/drivers/pinctrl/output-disable.c b/drivers/pinctrl/output-disable.c
new file mode 100644
index 000000000000..fb233b10b7df
--- /dev/null
+++ b/drivers/pinctrl/output-disable.c
@@ -0,0 +1,148 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Generic pin output-disable sysfs interface for pinctrl
+ * Copyright (C) 2023 <biju.das.jz@bp.renesas.com>
+ */
+
+#include <linux/debugfs.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/string.h>
+#include <linux/types.h>
+
+#include <linux/pinctrl/consumer.h>
+#include <linux/pinctrl/output-disable.h>
+#include <linux/pinctrl/pinctrl.h>
+
+#include "core.h"
+#include "output-disable.h"
+
+#define OUTPUT_DISABLE_BUF_MAX 128
+
+static int output_disable_get_vars(char *src, char **dst)
+{
+ char *result = (char *)dst;
+
+ /* find a separator which is a spacelike character */
+ for (result = src; !isspace(*result); result++) {
+ if (*result == '\0')
+ return -EINVAL;
+ }
+ *result = '\0';
+
+ /* drop extra spaces between function and group names */
+ result = skip_spaces(result + 1);
+ if (*result == '\0')
+ return -EINVAL;
+
+ *dst = result;
+
+ return 0;
+}
+
+static ssize_t configure_store(struct device *parent,
+ struct device_attribute *attr,
+ const char *src_buf,
+ size_t len)
+{
+ struct pinctrl_dev *pctldev = dev_get_drvdata(parent);
+ const struct pin_output_disable_ops *poutdisops = pctldev->desc->poutdisops;
+ char *buf, *gname, *fname, *conf, *conf_val;
+ int config, config_val;
+ struct device *dev;
+ int ret;
+
+ if (len > OUTPUT_DISABLE_BUF_MAX)
+ return -ENOMEM;
+
+ buf = kzalloc(OUTPUT_DISABLE_BUF_MAX, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ memcpy(buf, src_buf, len);
+ buf[len - 1] = '\0';
+
+ /* remove leading and trailing spaces of input buffer */
+ gname = strstrip(buf);
+ if (*gname == '\0') {
+ ret = -EINVAL;
+ goto exit_free_buf;
+ }
+
+ ret = output_disable_get_vars(gname, &fname);
+ if (ret < 0)
+ goto exit_free_buf;
+
+ ret = output_disable_get_vars(fname, &conf);
+ if (ret < 0)
+ goto exit_free_buf;
+
+ ret = output_disable_get_vars(conf, &conf_val);
+ if (ret < 0)
+ goto exit_free_buf;
+
+ ret = kstrtoint(conf, 0, &config);
+ if (ret)
+ goto exit_free_buf;
+
+ ret = kstrtoint(conf_val, 0, &config_val);
+ if (ret)
+ goto exit_free_buf;
+
+ dev = pinctrl_get_device(fname, gname);
+ if (IS_ERR(dev)) {
+ dev_err(pctldev->dev, "invalid function %s in map table\n", fname);
+ goto exit_free_buf;
+ }
+
+ if (poutdisops)
+ poutdisops->pin_output_disable_config_set(pctldev, dev,
+ fname, gname,
+ config, config_val);
+
+exit_free_buf:
+ kfree(buf);
+
+ return ret ? : len;
+}
+static DEVICE_ATTR_WO(configure);
+
+static struct attribute *pinctrl_attrs[] = {
+ &dev_attr_configure.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(pinctrl);
+
+static struct class pinctrl_class = {
+ .name = "pinctrl",
+ .owner = THIS_MODULE,
+ .dev_groups = pinctrl_groups,
+};
+
+void output_disable_init_device_sysfs(struct pinctrl_dev *pctldev)
+{
+ struct device *parent;
+
+ parent = device_create(&pinctrl_class, pctldev->dev, MKDEV(0, 0),
+ pctldev, "output-disable");
+ if (IS_ERR(parent))
+ dev_err(pctldev->dev, "device_create failed for pinctrl sysfs\n");
+}
+
+void output_disable_remove_device_sysfs(struct pinctrl_dev *pctldev)
+{
+ device_destroy(&pinctrl_class, MKDEV(0, 0));
+}
+
+static int __init pinctrl_sysfs_init(void)
+{
+ return class_register(&pinctrl_class);
+}
+subsys_initcall(pinctrl_sysfs_init);
+
+static void __exit pinctrl_sysfs_exit(void)
+{
+ class_unregister(&pinctrl_class);
+}
+module_exit(pinctrl_sysfs_exit);
diff --git a/drivers/pinctrl/output-disable.h b/drivers/pinctrl/output-disable.h
new file mode 100644
index 000000000000..f933965599d2
--- /dev/null
+++ b/drivers/pinctrl/output-disable.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Internal interface between the core pin control system and the
+ * ouput-disable portions
+ *
+ * Copyright (C) 2023 Renesas Electronics Corporation
+ * Based on bits of pinmux.c
+ *
+ * Author: Biju Das <biju.das.jz@bp.renesas.com>
+ */
+
+#include <linux/types.h>
+
+struct dentry;
+struct seq_file;
+
+struct pinctrl_dev;
+
+#ifdef CONFIG_PIN_OUTPUT_DISABLE
+void output_disable_init_device_sysfs(struct pinctrl_dev *pctldev);
+void output_disable_remove_device_sysfs(struct pinctrl_dev *pctldev);
+
+#else
+static inline void output_disable_init_device_sysfs(struct pinctrl_dev *pctldev)
+{
+}
+
+static inline void output_disable_remove_device_sysfs(struct pinctrl_dev *pctldev)
+{
+}
+
+#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH v6 03/13] pinctrl: Add sysfs support
2023-03-06 9:00 ` [PATCH v6 03/13] pinctrl: Add sysfs support Biju Das
@ 2023-03-06 23:38 ` andy.shevchenko
2023-03-07 8:54 ` Biju Das
2023-03-07 13:41 ` Linus Walleij
1 sibling, 1 reply; 44+ messages in thread
From: andy.shevchenko @ 2023-03-06 23:38 UTC (permalink / raw)
To: Biju Das
Cc: Linus Walleij, linux-gpio, Geert Uytterhoeven, Thierry Reding,
Uwe Kleine-König, Prabhakar Mahadev Lad, linux-renesas-soc
Mon, Mar 06, 2023 at 09:00:04AM +0000, Biju Das kirjoitti:
> Add a simple sysfs interface to the generic pinctrl framework
> for configuring pins for output disable operation.
>
> /sys/class/pinctrl/
> `-- output-disable/
> |-- configure (w/o) ask the kernel to configure a pin group
> for output disable operation.
>
> echo "<group-name function-name 0 1>" > configure
>
> The existing "pinmux-functions" debugfs file lists the pin functions
> registered for the pin controller. For example:
>
> function 0: usb0, groups = [ usb0 ]
> function 1: usb1, groups = [ usb1 ]
> function 2: gpt4-pins, groups = [ gpt4-pins ]
> function 3: scif0, groups = [ scif0 ]
> function 4: scif2, groups = [ scif2 ]
> function 5: spi1, groups = [ spi1 ]
>
> To configure gpt4-pins for output disable activation by user:
>
> echo "gpt4-pins gpt4-pins 0 1" > configure
...
> +static struct attribute *pinctrl_attrs[] = {
> + &dev_attr_configure.attr,
> + NULL,
No comma for a terminator entry.
> +};
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 44+ messages in thread* RE: [PATCH v6 03/13] pinctrl: Add sysfs support
2023-03-06 23:38 ` andy.shevchenko
@ 2023-03-07 8:54 ` Biju Das
0 siblings, 0 replies; 44+ messages in thread
From: Biju Das @ 2023-03-07 8:54 UTC (permalink / raw)
To: andy.shevchenko@gmail.com
Cc: Linus Walleij, linux-gpio@vger.kernel.org, Geert Uytterhoeven,
Thierry Reding, Uwe Kleine-König, Prabhakar Mahadev Lad,
linux-renesas-soc@vger.kernel.org
Hi Andy Shevchenko,
Thanks for the feedback.
> Subject: Re: [PATCH v6 03/13] pinctrl: Add sysfs support
>
> Mon, Mar 06, 2023 at 09:00:04AM +0000, Biju Das kirjoitti:
> > Add a simple sysfs interface to the generic pinctrl framework for
> > configuring pins for output disable operation.
> >
> > /sys/class/pinctrl/
> > `-- output-disable/
> > |-- configure (w/o) ask the kernel to configure a pin group
> > for output disable operation.
> >
> > echo "<group-name function-name 0 1>" > configure
> >
> > The existing "pinmux-functions" debugfs file lists the pin functions
> > registered for the pin controller. For example:
> >
> > function 0: usb0, groups = [ usb0 ]
> > function 1: usb1, groups = [ usb1 ]
> > function 2: gpt4-pins, groups = [ gpt4-pins ]
> > function 3: scif0, groups = [ scif0 ]
> > function 4: scif2, groups = [ scif2 ]
> > function 5: spi1, groups = [ spi1 ]
> >
> > To configure gpt4-pins for output disable activation by user:
> >
> > echo "gpt4-pins gpt4-pins 0 1" > configure
>
> ...
>
> > +static struct attribute *pinctrl_attrs[] = {
> > + &dev_attr_configure.attr,
> > + NULL,
>
> No comma for a terminator entry.
Agreed.
Cheers,
Biju
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v6 03/13] pinctrl: Add sysfs support
2023-03-06 9:00 ` [PATCH v6 03/13] pinctrl: Add sysfs support Biju Das
2023-03-06 23:38 ` andy.shevchenko
@ 2023-03-07 13:41 ` Linus Walleij
2023-03-07 14:43 ` Biju Das
2023-03-07 14:46 ` Greg KH
1 sibling, 2 replies; 44+ messages in thread
From: Linus Walleij @ 2023-03-07 13:41 UTC (permalink / raw)
To: Biju Das, Greg KH
Cc: linux-gpio, Geert Uytterhoeven, Thierry Reding,
Uwe Kleine-König, Prabhakar Mahadev Lad, linux-renesas-soc
On Mon, Mar 6, 2023 at 10:00 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Add a simple sysfs interface to the generic pinctrl framework
> for configuring pins for output disable operation.
>
> /sys/class/pinctrl/
> `-- output-disable/
> |-- configure (w/o) ask the kernel to configure a pin group
> for output disable operation.
>
> echo "<group-name function-name 0 1>" > configure
>
> The existing "pinmux-functions" debugfs file lists the pin functions
> registered for the pin controller. For example:
>
> function 0: usb0, groups = [ usb0 ]
> function 1: usb1, groups = [ usb1 ]
> function 2: gpt4-pins, groups = [ gpt4-pins ]
> function 3: scif0, groups = [ scif0 ]
> function 4: scif2, groups = [ scif2 ]
> function 5: spi1, groups = [ spi1 ]
>
> To configure gpt4-pins for output disable activation by user:
>
> echo "gpt4-pins gpt4-pins 0 1" > configure
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
You have to run things like this by Greg K-H (adde on To)
> +static struct class pinctrl_class = {
> + .name = "pinctrl",
> + .owner = THIS_MODULE,
> + .dev_groups = pinctrl_groups,
> +};
Why are you adding a new device class?
IIRC Greg don't like them, we should probably just deal with the
devices directly on the bus where they are, which also implies
some topology search etc which is a feature.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 44+ messages in thread* RE: [PATCH v6 03/13] pinctrl: Add sysfs support
2023-03-07 13:41 ` Linus Walleij
@ 2023-03-07 14:43 ` Biju Das
2023-03-07 15:15 ` Greg KH
2023-03-07 14:46 ` Greg KH
1 sibling, 1 reply; 44+ messages in thread
From: Biju Das @ 2023-03-07 14:43 UTC (permalink / raw)
To: Linus Walleij, Greg KH
Cc: linux-gpio@vger.kernel.org, Geert Uytterhoeven, Thierry Reding,
Uwe Kleine-König, Prabhakar Mahadev Lad,
linux-renesas-soc@vger.kernel.org
Hi Linus Walleij,
Thanks for the feedback.
> Subject: Re: [PATCH v6 03/13] pinctrl: Add sysfs support
>
> On Mon, Mar 6, 2023 at 10:00 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
>
> > Add a simple sysfs interface to the generic pinctrl framework for
> > configuring pins for output disable operation.
> >
> > /sys/class/pinctrl/
> > `-- output-disable/
> > |-- configure (w/o) ask the kernel to configure a pin group
> > for output disable operation.
> >
> > echo "<group-name function-name 0 1>" > configure
> >
> > The existing "pinmux-functions" debugfs file lists the pin functions
> > registered for the pin controller. For example:
> >
> > function 0: usb0, groups = [ usb0 ]
> > function 1: usb1, groups = [ usb1 ]
> > function 2: gpt4-pins, groups = [ gpt4-pins ]
> > function 3: scif0, groups = [ scif0 ]
> > function 4: scif2, groups = [ scif2 ]
> > function 5: spi1, groups = [ spi1 ]
> >
> > To configure gpt4-pins for output disable activation by user:
> >
> > echo "gpt4-pins gpt4-pins 0 1" > configure
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>
>
> You have to run things like this by Greg K-H (adde on To)
>
> > +static struct class pinctrl_class = {
> > + .name = "pinctrl",
> > + .owner = THIS_MODULE,
> > + .dev_groups = pinctrl_groups,
> > +};
>
> Why are you adding a new device class?
>
> IIRC Greg don't like them, we should probably just deal with the devices
> directly on the bus where they are, which also implies some topology search
> etc which is a feature.
I am ok for both,
I thought adding new device class will be more generic and
people can use simple sysfs[1] like pwm for output disable operation
rather than the SoC specific operation[2].
[1]
/sys/class/pinctrl/output-disable/configure
vs
[2]
/sys/devices/platform/soc/11030000.pinctrl/output_disable
Cheers,
Biju
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v6 03/13] pinctrl: Add sysfs support
2023-03-07 14:43 ` Biju Das
@ 2023-03-07 15:15 ` Greg KH
2023-03-28 7:07 ` Biju Das
0 siblings, 1 reply; 44+ messages in thread
From: Greg KH @ 2023-03-07 15:15 UTC (permalink / raw)
To: Biju Das
Cc: Linus Walleij, linux-gpio@vger.kernel.org, Geert Uytterhoeven,
Thierry Reding, Uwe Kleine-König, Prabhakar Mahadev Lad,
linux-renesas-soc@vger.kernel.org
On Tue, Mar 07, 2023 at 02:43:54PM +0000, Biju Das wrote:
> Hi Linus Walleij,
>
> Thanks for the feedback.
>
> > Subject: Re: [PATCH v6 03/13] pinctrl: Add sysfs support
> >
> > On Mon, Mar 6, 2023 at 10:00 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> >
> > > Add a simple sysfs interface to the generic pinctrl framework for
> > > configuring pins for output disable operation.
> > >
> > > /sys/class/pinctrl/
> > > `-- output-disable/
> > > |-- configure (w/o) ask the kernel to configure a pin group
> > > for output disable operation.
> > >
> > > echo "<group-name function-name 0 1>" > configure
> > >
> > > The existing "pinmux-functions" debugfs file lists the pin functions
> > > registered for the pin controller. For example:
> > >
> > > function 0: usb0, groups = [ usb0 ]
> > > function 1: usb1, groups = [ usb1 ]
> > > function 2: gpt4-pins, groups = [ gpt4-pins ]
> > > function 3: scif0, groups = [ scif0 ]
> > > function 4: scif2, groups = [ scif2 ]
> > > function 5: spi1, groups = [ spi1 ]
> > >
> > > To configure gpt4-pins for output disable activation by user:
> > >
> > > echo "gpt4-pins gpt4-pins 0 1" > configure
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >
> >
> > You have to run things like this by Greg K-H (adde on To)
> >
> > > +static struct class pinctrl_class = {
> > > + .name = "pinctrl",
> > > + .owner = THIS_MODULE,
> > > + .dev_groups = pinctrl_groups,
> > > +};
> >
> > Why are you adding a new device class?
> >
> > IIRC Greg don't like them, we should probably just deal with the devices
> > directly on the bus where they are, which also implies some topology search
> > etc which is a feature.
>
> I am ok for both,
>
> I thought adding new device class will be more generic and
> people can use simple sysfs[1] like pwm for output disable operation
> rather than the SoC specific operation[2].
>
> [1]
> /sys/class/pinctrl/output-disable/configure
That's fine, but you don't need a class for it, use configfs for
configuring things like this, that is what it is there for.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 44+ messages in thread* RE: [PATCH v6 03/13] pinctrl: Add sysfs support
2023-03-07 15:15 ` Greg KH
@ 2023-03-28 7:07 ` Biju Das
0 siblings, 0 replies; 44+ messages in thread
From: Biju Das @ 2023-03-28 7:07 UTC (permalink / raw)
To: Greg KH
Cc: Linus Walleij, linux-gpio@vger.kernel.org, Geert Uytterhoeven,
Thierry Reding, Uwe Kleine-König, Prabhakar Mahadev Lad,
linux-renesas-soc@vger.kernel.org
Hi Greg,
> Subject: Re: [PATCH v6 03/13] pinctrl: Add sysfs support
>
> On Tue, Mar 07, 2023 at 02:43:54PM +0000, Biju Das wrote:
> > Hi Linus Walleij,
> >
> > Thanks for the feedback.
> >
> > > Subject: Re: [PATCH v6 03/13] pinctrl: Add sysfs support
> > >
> > > On Mon, Mar 6, 2023 at 10:00 AM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > >
> > > > Add a simple sysfs interface to the generic pinctrl framework for
> > > > configuring pins for output disable operation.
> > > >
> > > > /sys/class/pinctrl/
> > > > `-- output-disable/
> > > > |-- configure (w/o) ask the kernel to configure a pin group
> > > > for output disable operation.
> > > >
> > > > echo "<group-name function-name 0 1>" > configure
> > > >
> > > > The existing "pinmux-functions" debugfs file lists the pin
> > > > functions registered for the pin controller. For example:
> > > >
> > > > function 0: usb0, groups = [ usb0 ]
> > > > function 1: usb1, groups = [ usb1 ]
> > > > function 2: gpt4-pins, groups = [ gpt4-pins ]
> > > > function 3: scif0, groups = [ scif0 ]
> > > > function 4: scif2, groups = [ scif2 ]
> > > > function 5: spi1, groups = [ spi1 ]
> > > >
> > > > To configure gpt4-pins for output disable activation by user:
> > > >
> > > > echo "gpt4-pins gpt4-pins 0 1" > configure
> > > >
> > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > >
> > >
> > > You have to run things like this by Greg K-H (adde on To)
> > >
> > > > +static struct class pinctrl_class = {
> > > > + .name = "pinctrl",
> > > > + .owner = THIS_MODULE,
> > > > + .dev_groups = pinctrl_groups, };
> > >
> > > Why are you adding a new device class?
> > >
> > > IIRC Greg don't like them, we should probably just deal with the
> > > devices directly on the bus where they are, which also implies some
> > > topology search etc which is a feature.
> >
> > I am ok for both,
> >
> > I thought adding new device class will be more generic and people can
> > use simple sysfs[1] like pwm for output disable operation rather than
> > the SoC specific operation[2].
> >
> > [1]
> > /sys/class/pinctrl/output-disable/configure
>
> That's fine, but you don't need a class for it, use configfs for configuring
> things like this, that is what it is there for.
As per discussion with Linus W and Geert, it is agreed to use configure this
settings in device tree as DT is the best place for system config like these use
cases.
Cheers,
Biju
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v6 03/13] pinctrl: Add sysfs support
2023-03-07 13:41 ` Linus Walleij
2023-03-07 14:43 ` Biju Das
@ 2023-03-07 14:46 ` Greg KH
1 sibling, 0 replies; 44+ messages in thread
From: Greg KH @ 2023-03-07 14:46 UTC (permalink / raw)
To: Linus Walleij
Cc: Biju Das, linux-gpio, Geert Uytterhoeven, Thierry Reding,
Uwe Kleine-König, Prabhakar Mahadev Lad, linux-renesas-soc
On Tue, Mar 07, 2023 at 02:41:16PM +0100, Linus Walleij wrote:
> On Mon, Mar 6, 2023 at 10:00 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
>
> > Add a simple sysfs interface to the generic pinctrl framework
> > for configuring pins for output disable operation.
> >
> > /sys/class/pinctrl/
> > `-- output-disable/
> > |-- configure (w/o) ask the kernel to configure a pin group
> > for output disable operation.
> >
> > echo "<group-name function-name 0 1>" > configure
> >
> > The existing "pinmux-functions" debugfs file lists the pin functions
> > registered for the pin controller. For example:
> >
> > function 0: usb0, groups = [ usb0 ]
> > function 1: usb1, groups = [ usb1 ]
> > function 2: gpt4-pins, groups = [ gpt4-pins ]
> > function 3: scif0, groups = [ scif0 ]
> > function 4: scif2, groups = [ scif2 ]
> > function 5: spi1, groups = [ spi1 ]
> >
> > To configure gpt4-pins for output disable activation by user:
> >
> > echo "gpt4-pins gpt4-pins 0 1" > configure
That is not what sysfs is for (multiple values per file.) Why not use
configfs for this as that is exactly what this is designed for?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v6 04/13] pinctrl: renesas: rzg2l: Add pin output disable support
2023-03-06 9:00 [PATCH v6 00/13] Add pinctrl sysfs and RZ/G2L POEG support Biju Das
` (2 preceding siblings ...)
2023-03-06 9:00 ` [PATCH v6 03/13] pinctrl: Add sysfs support Biju Das
@ 2023-03-06 9:00 ` Biju Das
2023-03-06 9:00 ` [PATCH v6 06/13] drivers: pinctrl: renesas: Add RZ/G2L POEG driver support Biju Das
` (7 subsequent siblings)
11 siblings, 0 replies; 44+ messages in thread
From: Biju Das @ 2023-03-06 9:00 UTC (permalink / raw)
To: Linus Walleij
Cc: Biju Das, linux-gpio, Geert Uytterhoeven, Thierry Reding,
Uwe Kleine-König, Prabhakar Mahadev Lad, linux-renesas-soc
Add pin output disable support for RZ/G2L alike SoCs by registering with
pincontrol core and provide callback mechanism for configuring pin output
disable device by matching with device registered with callback against
the device found by the core framework for a given function name and device
name.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v6:
* New patch
Ref:
https://lore.kernel.org/linux-renesas-soc/OS0PR01MB5922F5494D3C0862E15F3F8486B39@OS0PR01MB5922.jpnprd01.prod.outlook.com/T/#t
---
drivers/pinctrl/renesas/pinctrl-rzg2l.c | 44 +++++++++++++++++++++++++
include/linux/pinctrl/pinctrl-rzg2l.h | 26 +++++++++++++++
2 files changed, 70 insertions(+)
create mode 100644 include/linux/pinctrl/pinctrl-rzg2l.h
diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index 04b31f0c6b34..137d085077d8 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -17,9 +17,11 @@
#include <linux/spinlock.h>
#include <linux/pinctrl/consumer.h>
+#include <linux/pinctrl/output-disable.h>
#include <linux/pinctrl/pinconf-generic.h>
#include <linux/pinctrl/pinconf.h>
#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinctrl-rzg2l.h>
#include <linux/pinctrl/pinmux.h>
#include <dt-bindings/pinctrl/rzg2l-pinctrl.h>
@@ -136,6 +138,10 @@ struct rzg2l_pinctrl_data {
unsigned int n_dedicated_pins;
};
+static struct device *rzg2l_poeg_dev;
+static output_disable_cb rzg2l_poeg_cb;
+static void *rzg2l_poeg_context;
+
struct rzg2l_pinctrl {
struct pinctrl_dev *pctl;
struct pinctrl_desc desc;
@@ -747,6 +753,28 @@ static int rzg2l_pinctrl_pinconf_group_get(struct pinctrl_dev *pctldev,
return 0;
};
+static int rzg2l_pinctrl_output_disable_set(struct pinctrl_dev *pctldev,
+ struct device *dev,
+ const char *fname,
+ const char *gname,
+ enum pin_output_disable_conf conf,
+ unsigned int conf_val)
+{
+ struct rzg2l_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
+
+ if (!rzg2l_poeg_cb || !rzg2l_poeg_context) {
+ dev_err(pctrl->dev, "No callback function registered\n");
+ return -EINVAL;
+ }
+
+ if (!rzg2l_poeg_dev || rzg2l_poeg_dev != dev) {
+ dev_err(pctrl->dev, "Device match failed\n");
+ return -EINVAL;
+ }
+
+ return rzg2l_poeg_cb(rzg2l_poeg_context, fname, gname, conf, conf_val);
+}
+
static const struct pinctrl_ops rzg2l_pinctrl_pctlops = {
.get_groups_count = pinctrl_generic_get_group_count,
.get_group_name = pinctrl_generic_get_group_name,
@@ -772,6 +800,10 @@ static const struct pinconf_ops rzg2l_pinctrl_confops = {
.pin_config_config_dbg_show = pinconf_generic_dump_config,
};
+static const struct pin_output_disable_ops rzg2l_pinctrl_output_disable_fops = {
+ .pin_output_disable_config_set = rzg2l_pinctrl_output_disable_set,
+};
+
static int rzg2l_gpio_request(struct gpio_chip *chip, unsigned int offset)
{
struct rzg2l_pinctrl *pctrl = gpiochip_get_data(chip);
@@ -1320,6 +1352,17 @@ static void rzg2l_init_irq_valid_mask(struct gpio_chip *gc,
}
}
+int rzg2l_output_disable_cb_register(struct device *dev, output_disable_cb cb,
+ void *cb_context)
+{
+ rzg2l_poeg_dev = dev;
+ rzg2l_poeg_cb = cb;
+ rzg2l_poeg_context = cb_context;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(rzg2l_output_disable_cb_register);
+
static int rzg2l_gpio_register(struct rzg2l_pinctrl *pctrl)
{
struct device_node *np = pctrl->dev->of_node;
@@ -1404,6 +1447,7 @@ static int rzg2l_pinctrl_register(struct rzg2l_pinctrl *pctrl)
pctrl->desc.pctlops = &rzg2l_pinctrl_pctlops;
pctrl->desc.pmxops = &rzg2l_pinctrl_pmxops;
pctrl->desc.confops = &rzg2l_pinctrl_confops;
+ pctrl->desc.poutdisops = &rzg2l_pinctrl_output_disable_fops;
pctrl->desc.owner = THIS_MODULE;
pins = devm_kcalloc(pctrl->dev, pctrl->desc.npins, sizeof(*pins), GFP_KERNEL);
diff --git a/include/linux/pinctrl/pinctrl-rzg2l.h b/include/linux/pinctrl/pinctrl-rzg2l.h
new file mode 100644
index 000000000000..a49b4c5f8908
--- /dev/null
+++ b/include/linux/pinctrl/pinctrl-rzg2l.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __LINUX_PINCTRL_RZG2L_H__
+#define __LINUX_PINCTRL_RZG2L_H__
+
+#include <linux/pinctrl/output-disable.h>
+
+typedef int (*output_disable_cb) (void *context,
+ const char *fname,
+ const char *gname,
+ enum pin_output_disable_conf conf,
+ unsigned int conf_val);
+
+#if IS_ENABLED(CONFIG_PINCTRL_RZG2L)
+int rzg2l_output_disable_cb_register(struct device *dev,
+ output_disable_cb cb,
+ void *cb_context);
+#else
+static inline int rzg2l_output_disable_cb_register(struct device *dev,
+ output_disable_cb cb,
+ void *cb_context)
+{
+ return -EINVAL;
+}
+#endif
+
+#endif /* __LINUX_PINCTRL_RZG2L_H__ */
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread* [PATCH v6 06/13] drivers: pinctrl: renesas: Add RZ/G2L POEG driver support
2023-03-06 9:00 [PATCH v6 00/13] Add pinctrl sysfs and RZ/G2L POEG support Biju Das
` (3 preceding siblings ...)
2023-03-06 9:00 ` [PATCH v6 04/13] pinctrl: renesas: rzg2l: Add pin output disable support Biju Das
@ 2023-03-06 9:00 ` Biju Das
2023-03-06 23:35 ` andy.shevchenko
2023-03-06 9:00 ` [DO NOT APPLY PATCH v6 07/13] pwm: rzg2l-gpt: Add support for output disable request from gpt Biju Das
` (6 subsequent siblings)
11 siblings, 1 reply; 44+ messages in thread
From: Biju Das @ 2023-03-06 9:00 UTC (permalink / raw)
To: Linus Walleij, Philipp Zabel
Cc: Biju Das, Geert Uytterhoeven, Thierry Reding,
Uwe Kleine-König, linux-pwm, linux-renesas-soc, linux-gpio,
Chris Paterson, Prabhakar Mahadev Lad
The output pins of the RZ/G2L general PWM timer (GPT) can be disabled
by using the port output enabling function for the GPT (POEG).
Add basic support using s/w control through generic pincontrol sysfs to
enable/disable output from GPT by registering with RZ/G2L pincontrol
driver.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v5->v6:
* Dropped sysfs and is handled in generic driver.
v4->v5:
* Updated kernel version in sysfs doc.
v3->v4:
* Updated commit description.
v2->v3:
* Added sysfs documentation for output_disable
* PWM_RZG2L_GPT implies ARCH_RZG2L. So removed ARCH_RZG2L dependency
* Used dev_get_drvdata to get device data
* Replaced sprintf->sysfs_emit in show().
v1->v2:
* Renamed the file poeg-rzg2l->rzg2l-poeg
* Removed the macro POEGG as there is only single register and
updated rzg2l_poeg_write() and rzg2l_poeg_read()
* Updated error handling in probe()
Ref->v1:
* Moved driver files from soc to pincontrol directory
* Updated KConfig
---
drivers/pinctrl/renesas/Kconfig | 2 +
drivers/pinctrl/renesas/Makefile | 2 +
drivers/pinctrl/renesas/poeg/Kconfig | 11 +
drivers/pinctrl/renesas/poeg/Makefile | 2 +
drivers/pinctrl/renesas/poeg/rzg2l-poeg.c | 254 ++++++++++++++++++++++
5 files changed, 271 insertions(+)
create mode 100644 drivers/pinctrl/renesas/poeg/Kconfig
create mode 100644 drivers/pinctrl/renesas/poeg/Makefile
create mode 100644 drivers/pinctrl/renesas/poeg/rzg2l-poeg.c
diff --git a/drivers/pinctrl/renesas/Kconfig b/drivers/pinctrl/renesas/Kconfig
index 0903a0a41831..92bdc2e1e125 100644
--- a/drivers/pinctrl/renesas/Kconfig
+++ b/drivers/pinctrl/renesas/Kconfig
@@ -308,4 +308,6 @@ config PINCTRL_PFC_SHX3
bool "pin control support for SH-X3" if COMPILE_TEST
select PINCTRL_SH_FUNC_GPIO
+source "drivers/pinctrl/renesas/poeg/Kconfig"
+
endmenu
diff --git a/drivers/pinctrl/renesas/Makefile b/drivers/pinctrl/renesas/Makefile
index 558b30ce0dec..de1bb592fbf3 100644
--- a/drivers/pinctrl/renesas/Makefile
+++ b/drivers/pinctrl/renesas/Makefile
@@ -52,6 +52,8 @@ obj-$(CONFIG_PINCTRL_RZG2L) += pinctrl-rzg2l.o
obj-$(CONFIG_PINCTRL_RZN1) += pinctrl-rzn1.o
obj-$(CONFIG_PINCTRL_RZV2M) += pinctrl-rzv2m.o
+obj-$(CONFIG_POEG_RZG2L) += poeg/
+
ifeq ($(CONFIG_COMPILE_TEST),y)
CFLAGS_pfc-sh7203.o += -I$(srctree)/arch/sh/include/cpu-sh2a
CFLAGS_pfc-sh7264.o += -I$(srctree)/arch/sh/include/cpu-sh2a
diff --git a/drivers/pinctrl/renesas/poeg/Kconfig b/drivers/pinctrl/renesas/poeg/Kconfig
new file mode 100644
index 000000000000..306e8ae81cb2
--- /dev/null
+++ b/drivers/pinctrl/renesas/poeg/Kconfig
@@ -0,0 +1,11 @@
+# SPDX-License-Identifier: GPL-2.0
+config POEG_RZG2L
+ tristate "Renesas RZ/G2L poeg support"
+ depends on PWM_RZG2L_GPT || COMPILE_TEST
+ depends on HAS_IOMEM
+ help
+ This driver exposes the Port Output Enable for GPT(POEG) found
+ in Renesas RZ/G2L alike SoCs.
+
+ To compile this driver as a module, choose M here: the module
+ will be called rzg2l-poeg.
diff --git a/drivers/pinctrl/renesas/poeg/Makefile b/drivers/pinctrl/renesas/poeg/Makefile
new file mode 100644
index 000000000000..610bdd6182be
--- /dev/null
+++ b/drivers/pinctrl/renesas/poeg/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_POEG_RZG2L) += rzg2l-poeg.o
diff --git a/drivers/pinctrl/renesas/poeg/rzg2l-poeg.c b/drivers/pinctrl/renesas/poeg/rzg2l-poeg.c
new file mode 100644
index 000000000000..30f4352e257d
--- /dev/null
+++ b/drivers/pinctrl/renesas/poeg/rzg2l-poeg.c
@@ -0,0 +1,254 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Renesas RZ/G2L Port Output Enable for GPT (POEG) driver
+ *
+ * Copyright (C) 2022 Renesas Electronics Corporation
+ */
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/pinctrl/pinctrl-rzg2l.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/reset.h>
+
+#define POEGG_SSF BIT(3)
+
+#define RZG2L_POEG_MAX_INDEX 3
+
+#define RZG2L_GPT_MAX_HW_CHANNELS 8
+#define RZG2L_GPT_INVALID_CHANNEL 0xff
+
+struct rzg2l_poeg_chip {
+ struct device *gpt_dev;
+ struct reset_control *rstc;
+ void __iomem *mmio;
+ u8 index;
+ u8 gpt_channels[RZG2L_GPT_MAX_HW_CHANNELS];
+};
+
+static const char * const rzg2l_gpt_pins[] = {
+ "gpt0-pins",
+ "gpt1-pins",
+ "gpt2-pins",
+ "gpt3-pins",
+ "gpt4-pins",
+ "gpt5-pins",
+ "gpt6-pins",
+ "gpt7-pins",
+};
+
+static void rzg2l_poeg_write(struct rzg2l_poeg_chip *chip, u32 data)
+{
+ iowrite32(data, chip->mmio);
+}
+
+static u32 rzg2l_poeg_read(struct rzg2l_poeg_chip *chip)
+{
+ return ioread32(chip->mmio);
+}
+
+static int rzg2l_poeg_output_disable_user(struct rzg2l_poeg_chip *chip,
+ bool enable)
+{
+ u32 reg_val;
+
+ reg_val = rzg2l_poeg_read(chip);
+ if (enable)
+ reg_val |= POEGG_SSF;
+ else
+ reg_val &= ~POEGG_SSF;
+
+ rzg2l_poeg_write(chip, reg_val);
+
+ return 0;
+}
+
+static int rzg2l_poeg_cb(void *context, const char *fname, const char *gname,
+ enum pin_output_disable_conf conf,
+ unsigned int conf_val)
+{
+ bool pin_match = false;
+ int ret, i;
+
+ for (i = 0; i < RZG2L_GPT_MAX_HW_CHANNELS; i++) {
+ if ((!strcmp(rzg2l_gpt_pins[i], fname)) &&
+ (!strcmp(rzg2l_gpt_pins[i], gname))) {
+ pin_match = true;
+ break;
+ }
+ }
+
+ if (!pin_match)
+ return -EINVAL;
+
+ switch (conf) {
+ case PINCTRL_OUTPUT_DISABLE_BY_USER:
+ ret = rzg2l_poeg_output_disable_user(context, !!conf_val);
+ break;
+ case PINCTRL_OUTPUT_DISABLE_BY_SOC_ON_PIN_OUTPUT_HIGH:
+ case PINCTRL_OUTPUT_DISABLE_BY_SOC_ON_PIN_OUTPUT_LOW:
+ case PINCTRL_OUTPUT_DISABLE_BY_SOC_ON_DEAD_TIME_ERROR:
+ default:
+ return -EINVAL;
+ }
+
+ return ret;
+}
+
+static bool rzg2l_poeg_get_linked_gpt_channels(struct platform_device *pdev,
+ struct rzg2l_poeg_chip *chip,
+ struct device_node *gpt_np,
+ u8 poeg_id)
+{
+ struct of_phandle_args of_args;
+ bool ret = false;
+ unsigned int i;
+ u32 poeg_grp;
+ int cells;
+ int err;
+
+ cells = of_property_count_u32_elems(gpt_np, "renesas,poegs");
+ if (cells == -EINVAL)
+ return ret;
+
+ for (i = 0 ; i < RZG2L_GPT_MAX_HW_CHANNELS; i++)
+ chip->gpt_channels[i] = RZG2L_GPT_INVALID_CHANNEL;
+
+ cells >>= 1;
+ for (i = 0; i < cells; i++) {
+ err = of_parse_phandle_with_fixed_args(gpt_np,
+ "renesas,poegs", 1, i,
+ &of_args);
+ if (err) {
+ dev_err(&pdev->dev,
+ "Failed to parse 'renesas,poegs' property\n");
+ break;
+ }
+
+ if (of_args.args[0] >= RZG2L_GPT_MAX_HW_CHANNELS) {
+ dev_err(&pdev->dev, "Invalid channel %d >= %d\n",
+ of_args.args[0], RZG2L_GPT_MAX_HW_CHANNELS);
+ of_node_put(of_args.np);
+ break;
+ }
+
+ if (!of_property_read_u32(of_args.np, "renesas,poeg-id", &poeg_grp)) {
+ if (poeg_grp == poeg_id) {
+ chip->gpt_channels[poeg_grp] = poeg_id;
+ ret = true;
+ }
+ }
+
+ of_node_put(of_args.np);
+ }
+
+ return ret;
+}
+
+static const struct of_device_id rzg2l_poeg_of_table[] = {
+ { .compatible = "renesas,rzg2l-poeg", },
+ { /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, rzg2l_poeg_of_table);
+
+static void rzg2l_poeg_cleanup(void *data)
+{
+ struct rzg2l_poeg_chip *chip = data;
+
+ put_device(chip->gpt_dev);
+}
+
+static int rzg2l_poeg_probe(struct platform_device *pdev)
+{
+ struct platform_device *gpt_pdev = NULL;
+ struct rzg2l_poeg_chip *chip;
+ bool gpt_linked = false;
+ struct device_node *np;
+ u32 val;
+ int ret;
+
+ chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
+ if (!chip)
+ return -ENOMEM;
+
+ if (!of_property_read_u32(pdev->dev.of_node, "renesas,poeg-id", &val))
+ chip->index = val;
+
+ if (chip->index > RZG2L_POEG_MAX_INDEX)
+ return -EINVAL;
+
+ np = of_parse_phandle(pdev->dev.of_node, "renesas,gpt", 0);
+ if (np)
+ gpt_pdev = of_find_device_by_node(np);
+
+ gpt_linked = rzg2l_poeg_get_linked_gpt_channels(pdev, chip, np,
+ chip->index);
+ of_node_put(np);
+ if (!gpt_pdev)
+ return -ENODEV;
+
+ chip->gpt_dev = &gpt_pdev->dev;
+ ret = devm_add_action_or_reset(&pdev->dev,
+ rzg2l_poeg_cleanup, chip);
+ if (ret < 0)
+ return ret;
+
+ chip->mmio = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(chip->mmio))
+ return PTR_ERR(chip->mmio);
+
+ if (gpt_linked)
+ rzg2l_output_disable_cb_register(chip->gpt_dev,
+ rzg2l_poeg_cb, chip);
+
+ chip->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
+ if (IS_ERR(chip->rstc))
+ return dev_err_probe(&pdev->dev, PTR_ERR(chip->rstc),
+ "get reset failed\n");
+
+ ret = reset_control_deassert(chip->rstc);
+ if (ret)
+ return ret;
+
+ platform_set_drvdata(pdev, chip);
+ pm_runtime_enable(&pdev->dev);
+ ret = pm_runtime_resume_and_get(&pdev->dev);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "pm_runtime_resume_and_get failed: %d\n", ret);
+ goto err_pm_disable;
+ }
+
+ return 0;
+
+err_pm_disable:
+ pm_runtime_disable(&pdev->dev);
+ reset_control_assert(chip->rstc);
+ return ret;
+}
+
+static int rzg2l_poeg_remove(struct platform_device *pdev)
+{
+ struct rzg2l_poeg_chip *chip = platform_get_drvdata(pdev);
+
+ pm_runtime_put(&pdev->dev);
+ pm_runtime_disable(&pdev->dev);
+ reset_control_assert(chip->rstc);
+
+ return 0;
+}
+
+static struct platform_driver rzg2l_poeg_driver = {
+ .driver = {
+ .name = "rzg2l-poeg",
+ .of_match_table = of_match_ptr(rzg2l_poeg_of_table),
+ },
+ .probe = rzg2l_poeg_probe,
+ .remove = rzg2l_poeg_remove,
+};
+module_platform_driver(rzg2l_poeg_driver);
+
+MODULE_AUTHOR("Biju Das <biju.das.jz@bp.renesas.com>");
+MODULE_DESCRIPTION("Renesas RZ/G2L POEG Driver");
+MODULE_LICENSE("GPL");
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH v6 06/13] drivers: pinctrl: renesas: Add RZ/G2L POEG driver support
2023-03-06 9:00 ` [PATCH v6 06/13] drivers: pinctrl: renesas: Add RZ/G2L POEG driver support Biju Das
@ 2023-03-06 23:35 ` andy.shevchenko
2023-03-07 8:53 ` Biju Das
0 siblings, 1 reply; 44+ messages in thread
From: andy.shevchenko @ 2023-03-06 23:35 UTC (permalink / raw)
To: Biju Das
Cc: Linus Walleij, Philipp Zabel, Geert Uytterhoeven, Thierry Reding,
Uwe Kleine-König, linux-pwm, linux-renesas-soc, linux-gpio,
Chris Paterson, Prabhakar Mahadev Lad
Mon, Mar 06, 2023 at 09:00:07AM +0000, Biju Das kirjoitti:
> The output pins of the RZ/G2L general PWM timer (GPT) can be disabled
> by using the port output enabling function for the GPT (POEG).
>
> Add basic support using s/w control through generic pincontrol sysfs to
> enable/disable output from GPT by registering with RZ/G2L pincontrol
> driver.
You have wrong Subject prefix.
...
> +static void rzg2l_poeg_write(struct rzg2l_poeg_chip *chip, u32 data)
> +{
> + iowrite32(data, chip->mmio);
> +}
> +
> +static u32 rzg2l_poeg_read(struct rzg2l_poeg_chip *chip)
> +{
> + return ioread32(chip->mmio);
> +}
Why not regmap MMIO?
...
> +static struct platform_driver rzg2l_poeg_driver = {
> + .driver = {
> + .name = "rzg2l-poeg",
> + .of_match_table = of_match_ptr(rzg2l_poeg_of_table),
Why do you need of_match_ptr()?
> + },
> + .probe = rzg2l_poeg_probe,
> + .remove = rzg2l_poeg_remove,
> +};
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 44+ messages in thread* RE: [PATCH v6 06/13] drivers: pinctrl: renesas: Add RZ/G2L POEG driver support
2023-03-06 23:35 ` andy.shevchenko
@ 2023-03-07 8:53 ` Biju Das
2023-03-07 9:58 ` Andy Shevchenko
0 siblings, 1 reply; 44+ messages in thread
From: Biju Das @ 2023-03-07 8:53 UTC (permalink / raw)
To: andy.shevchenko@gmail.com
Cc: Linus Walleij, Philipp Zabel, Geert Uytterhoeven, Thierry Reding,
Uwe Kleine-König, linux-pwm@vger.kernel.org,
linux-renesas-soc@vger.kernel.org, linux-gpio@vger.kernel.org,
Chris Paterson, Prabhakar Mahadev Lad
Hi Andy Shevchenko,
Thanks for the feedback.
> -----Original Message-----
> From: andy.shevchenko@gmail.com <andy.shevchenko@gmail.com>
> Sent: Monday, March 6, 2023 11:36 PM
> To: Biju Das <biju.das.jz@bp.renesas.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>; Philipp Zabel
> <p.zabel@pengutronix.de>; Geert Uytterhoeven <geert+renesas@glider.be>;
> Thierry Reding <thierry.reding@gmail.com>; Uwe Kleine-König <u.kleine-
> koenig@pengutronix.de>; linux-pwm@vger.kernel.org; linux-renesas-
> soc@vger.kernel.org; linux-gpio@vger.kernel.org; Chris Paterson
> <Chris.Paterson2@renesas.com>; Prabhakar Mahadev Lad <prabhakar.mahadev-
> lad.rj@bp.renesas.com>
> Subject: Re: [PATCH v6 06/13] drivers: pinctrl: renesas: Add RZ/G2L POEG
> driver support
>
> Mon, Mar 06, 2023 at 09:00:07AM +0000, Biju Das kirjoitti:
> > The output pins of the RZ/G2L general PWM timer (GPT) can be disabled
> > by using the port output enabling function for the GPT (POEG).
> >
> > Add basic support using s/w control through generic pincontrol sysfs
> > to enable/disable output from GPT by registering with RZ/G2L
> > pincontrol driver.
>
> You have wrong Subject prefix.
Oops. Will fix.
>
> ...
>
> > +static void rzg2l_poeg_write(struct rzg2l_poeg_chip *chip, u32 data)
> > +{
> > + iowrite32(data, chip->mmio);
> > +}
> > +
> > +static u32 rzg2l_poeg_read(struct rzg2l_poeg_chip *chip) {
> > + return ioread32(chip->mmio);
> > +}
>
> Why not regmap MMIO?
Some drivers used iowrite32, some uses writel, some uses regmap.
will use regmap for read/write,If the preference is regmap MMIO
as it comes with spinlock for MMIO access.
>
> ...
>
> > +static struct platform_driver rzg2l_poeg_driver = {
> > + .driver = {
> > + .name = "rzg2l-poeg",
> > + .of_match_table = of_match_ptr(rzg2l_poeg_of_table),
>
> Why do you need of_match_ptr()?
Not needed. Will remove it.
Cheers,
Biju
>
> > + },
> > + .probe = rzg2l_poeg_probe,
> > + .remove = rzg2l_poeg_remove,
> > +};
>
> --
> With Best Regards,
> Andy Shevchenko
>
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v6 06/13] drivers: pinctrl: renesas: Add RZ/G2L POEG driver support
2023-03-07 8:53 ` Biju Das
@ 2023-03-07 9:58 ` Andy Shevchenko
2023-03-07 10:10 ` Biju Das
0 siblings, 1 reply; 44+ messages in thread
From: Andy Shevchenko @ 2023-03-07 9:58 UTC (permalink / raw)
To: Biju Das
Cc: Linus Walleij, Philipp Zabel, Geert Uytterhoeven, Thierry Reding,
Uwe Kleine-König, linux-pwm@vger.kernel.org,
linux-renesas-soc@vger.kernel.org, linux-gpio@vger.kernel.org,
Chris Paterson, Prabhakar Mahadev Lad
On Tue, Mar 7, 2023 at 10:53 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > From: andy.shevchenko@gmail.com <andy.shevchenko@gmail.com>
> > Sent: Monday, March 6, 2023 11:36 PM
> > Mon, Mar 06, 2023 at 09:00:07AM +0000, Biju Das kirjoitti:
...
> > > +static void rzg2l_poeg_write(struct rzg2l_poeg_chip *chip, u32 data)
> > > +{
> > > + iowrite32(data, chip->mmio);
> > > +}
> > > +
> > > +static u32 rzg2l_poeg_read(struct rzg2l_poeg_chip *chip) {
> > > + return ioread32(chip->mmio);
> > > +}
> >
> > Why not regmap MMIO?
>
> Some drivers used iowrite32, some uses writel, some uses regmap.
>
> will use regmap for read/write,If the preference is regmap MMIO
> as it comes with spinlock for MMIO access.
Lock can be disabled. It's up to the user of regmap.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 44+ messages in thread* RE: [PATCH v6 06/13] drivers: pinctrl: renesas: Add RZ/G2L POEG driver support
2023-03-07 9:58 ` Andy Shevchenko
@ 2023-03-07 10:10 ` Biju Das
2023-03-07 12:30 ` Andy Shevchenko
0 siblings, 1 reply; 44+ messages in thread
From: Biju Das @ 2023-03-07 10:10 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Linus Walleij, Philipp Zabel, Geert Uytterhoeven, Thierry Reding,
Uwe Kleine-König, linux-pwm@vger.kernel.org,
linux-renesas-soc@vger.kernel.org, linux-gpio@vger.kernel.org,
Chris Paterson, Prabhakar Mahadev Lad
Hi Andy Shevchenko,
Thanks for the feedback.
> Subject: Re: [PATCH v6 06/13] drivers: pinctrl: renesas: Add RZ/G2L POEG
> driver support
>
> On Tue, Mar 7, 2023 at 10:53 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > From: andy.shevchenko@gmail.com <andy.shevchenko@gmail.com>
> > > Sent: Monday, March 6, 2023 11:36 PM Mon, Mar 06, 2023 at 09:00:07AM
> > > +0000, Biju Das kirjoitti:
>
> ...
>
> > > > +static void rzg2l_poeg_write(struct rzg2l_poeg_chip *chip, u32
> > > > +data) {
> > > > + iowrite32(data, chip->mmio);
> > > > +}
> > > > +
> > > > +static u32 rzg2l_poeg_read(struct rzg2l_poeg_chip *chip) {
> > > > + return ioread32(chip->mmio);
> > > > +}
> > >
> > > Why not regmap MMIO?
> >
> > Some drivers used iowrite32, some uses writel, some uses regmap.
> >
> > will use regmap for read/write,If the preference is regmap MMIO as it
> > comes with spinlock for MMIO access.
>
> Lock can be disabled. It's up to the user of regmap.
Ok, Just want to double check,
POEG has a single 32 bit register. So it worth to use regmap?
A simple readl/write is sufficient no??
Cheers,
Biju
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v6 06/13] drivers: pinctrl: renesas: Add RZ/G2L POEG driver support
2023-03-07 10:10 ` Biju Das
@ 2023-03-07 12:30 ` Andy Shevchenko
2023-03-07 12:39 ` Biju Das
0 siblings, 1 reply; 44+ messages in thread
From: Andy Shevchenko @ 2023-03-07 12:30 UTC (permalink / raw)
To: Biju Das
Cc: Linus Walleij, Philipp Zabel, Geert Uytterhoeven, Thierry Reding,
Uwe Kleine-König, linux-pwm@vger.kernel.org,
linux-renesas-soc@vger.kernel.org, linux-gpio@vger.kernel.org,
Chris Paterson, Prabhakar Mahadev Lad
On Tue, Mar 7, 2023 at 12:10 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > On Tue, Mar 7, 2023 at 10:53 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > > From: andy.shevchenko@gmail.com <andy.shevchenko@gmail.com>
> > > > Sent: Monday, March 6, 2023 11:36 PM Mon, Mar 06, 2023 at 09:00:07AM
> > > > +0000, Biju Das kirjoitti:
...
> > > > > +static void rzg2l_poeg_write(struct rzg2l_poeg_chip *chip, u32
> > > > > +data) {
> > > > > + iowrite32(data, chip->mmio);
> > > > > +}
> > > > > +
> > > > > +static u32 rzg2l_poeg_read(struct rzg2l_poeg_chip *chip) {
> > > > > + return ioread32(chip->mmio);
> > > > > +}
> > > >
> > > > Why not regmap MMIO?
> > >
> > > Some drivers used iowrite32, some uses writel, some uses regmap.
> > >
> > > will use regmap for read/write,If the preference is regmap MMIO as it
> > > comes with spinlock for MMIO access.
> >
> > Lock can be disabled. It's up to the user of regmap.
>
> Ok, Just want to double check,
> POEG has a single 32 bit register. So it worth to use regmap?
> A simple readl/write is sufficient no??
It can be. But can you explain why you used iowriteXX() / ioreadXX()
instead of writeX()/readX()?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 44+ messages in thread* RE: [PATCH v6 06/13] drivers: pinctrl: renesas: Add RZ/G2L POEG driver support
2023-03-07 12:30 ` Andy Shevchenko
@ 2023-03-07 12:39 ` Biju Das
0 siblings, 0 replies; 44+ messages in thread
From: Biju Das @ 2023-03-07 12:39 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Linus Walleij, Philipp Zabel, Geert Uytterhoeven, Thierry Reding,
Uwe Kleine-König, linux-pwm@vger.kernel.org,
linux-renesas-soc@vger.kernel.org, linux-gpio@vger.kernel.org,
Chris Paterson, Prabhakar Mahadev Lad
Hi Andy Shevchenko,
Thanks for the feedback.
> Subject: Re: [PATCH v6 06/13] drivers: pinctrl: renesas: Add RZ/G2L POEG
> driver support
>
> On Tue, Mar 7, 2023 at 12:10 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > On Tue, Mar 7, 2023 at 10:53 AM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > > > > From: andy.shevchenko@gmail.com <andy.shevchenko@gmail.com>
> > > > > Sent: Monday, March 6, 2023 11:36 PM Mon, Mar 06, 2023 at
> > > > > 09:00:07AM
> > > > > +0000, Biju Das kirjoitti:
>
> ...
>
> > > > > > +static void rzg2l_poeg_write(struct rzg2l_poeg_chip *chip,
> > > > > > +u32
> > > > > > +data) {
> > > > > > + iowrite32(data, chip->mmio); }
> > > > > > +
> > > > > > +static u32 rzg2l_poeg_read(struct rzg2l_poeg_chip *chip) {
> > > > > > + return ioread32(chip->mmio); }
> > > > >
> > > > > Why not regmap MMIO?
> > > >
> > > > Some drivers used iowrite32, some uses writel, some uses regmap.
> > > >
> > > > will use regmap for read/write,If the preference is regmap MMIO as
> > > > it comes with spinlock for MMIO access.
> > >
> > > Lock can be disabled. It's up to the user of regmap.
> >
> > Ok, Just want to double check,
> > POEG has a single 32 bit register. So it worth to use regmap?
> > A simple readl/write is sufficient no??
>
> It can be. But can you explain why you used iowriteXX() / ioreadXX() instead
> of writeX()/readX()?
It is a mistake from my side. I have referred RZ/G2L BSP poeg driver code[1],
initially and missed to change this.
[1]
https://github.com/renesas-rz/rz_linux-cip/blob/rz-5.10-cip17/drivers/pwm/poeg-rzg2l.c
Cheers,
Biju
^ permalink raw reply [flat|nested] 44+ messages in thread
* [DO NOT APPLY PATCH v6 07/13] pwm: rzg2l-gpt: Add support for output disable request from gpt
2023-03-06 9:00 [PATCH v6 00/13] Add pinctrl sysfs and RZ/G2L POEG support Biju Das
` (4 preceding siblings ...)
2023-03-06 9:00 ` [PATCH v6 06/13] drivers: pinctrl: renesas: Add RZ/G2L POEG driver support Biju Das
@ 2023-03-06 9:00 ` Biju Das
2023-03-06 9:00 ` [DO NOT APPLY PATCH v6 08/13] pinctrl: renesas: rzg2l-poeg: Add support for GPT Output-Disable Request Biju Das
` (5 subsequent siblings)
11 siblings, 0 replies; 44+ messages in thread
From: Biju Das @ 2023-03-06 9:00 UTC (permalink / raw)
To: Linus Walleij, Thierry Reding
Cc: Biju Das, Uwe Kleine-König, Geert Uytterhoeven, Magnus Damm,
linux-pwm, linux-renesas-soc, linux-gpio, Prabhakar Mahadev Lad
When dead time error occurs or the GTIOCA pin output value is
the same as the GTIOCB pin output value, output protection is
required. GPT detects this condition and generates output disable
requests to POEG based on the settings in the output disable request
permission bits, such as GTINTAD.GRPDTE, GTINTAD.GRPABH,
GTINTAD.GRPABL. After the POEG receives output disable requests from
each channel and calculates external input using an OR operation, the
POEG generates output disable requests to GPT.
This patch adds support for output disable request from gpt,
when same time output level is high.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
drivers/pwm/pwm-rzg2l-gpt.c | 111 ++++++++++++++++++++++++++++++++++
include/linux/pwm/rzg2l-gpt.h | 32 ++++++++++
2 files changed, 143 insertions(+)
create mode 100644 include/linux/pwm/rzg2l-gpt.h
diff --git a/drivers/pwm/pwm-rzg2l-gpt.c b/drivers/pwm/pwm-rzg2l-gpt.c
index 9f3e2f7635a8..2f138e95f752 100644
--- a/drivers/pwm/pwm-rzg2l-gpt.c
+++ b/drivers/pwm/pwm-rzg2l-gpt.c
@@ -25,6 +25,7 @@
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
#include <linux/pwm.h>
+#include <linux/pwm/rzg2l-gpt.h>
#include <linux/reset.h>
#include <linux/time.h>
@@ -32,6 +33,7 @@
#define RZG2L_GTUDDTYC 0x30
#define RZG2L_GTIOR 0x34
#define RZG2L_GTINTAD 0x38
+#define RZG2L_GTST 0x3c
#define RZG2L_GTBER 0x40
#define RZG2L_GTCNT 0x48
#define RZG2L_GTCCRA 0x4c
@@ -72,6 +74,12 @@
(FIELD_PREP(RZG2L_GTIOR_GTIOB, RZG2L_INIT_OUT_LO_OUT_LO_END_TOGGLE) | RZG2L_GTIOR_OBE)
#define RZG2L_GTINTAD_GRP_MASK GENMASK(25, 24)
+#define RZG2L_GTINTAD_OUTPUT_DISABLE_SAME_LEVEL_HIGH BIT(29)
+
+#define RZG2L_GTST_OABHF BIT(29)
+#define RZG2L_GTST_OABLF BIT(30)
+
+#define RZG2L_GTST_POEG_IRQ_MASK GENMASK(30, 28)
#define RZG2L_GTCCR(i) (0x4c + 4 * (i))
@@ -431,6 +439,109 @@ static DEFINE_RUNTIME_DEV_PM_OPS(rzg2l_gpt_pm_ops,
rzg2l_gpt_pm_runtime_suspend,
rzg2l_gpt_pm_runtime_resume, NULL);
+u32 rzg2l_gpt_poeg_disable_req_irq_status(void *dev, u8 grp)
+{
+ u8 bitpos = grp * RZG2L_MAX_HW_CHANNELS;
+ struct rzg2l_gpt_chip *rzg2l_gpt;
+ unsigned int i;
+ u32 val = 0;
+ u32 offs;
+ u32 reg;
+
+ rzg2l_gpt = dev_get_drvdata(dev);
+ for (i = 0; i < RZG2L_MAX_HW_CHANNELS; i++) {
+ val <<= 3;
+ if (!test_bit(bitpos + i, rzg2l_gpt->poeg_gpt_link))
+ continue;
+
+ offs = RZG2L_GET_CH_OFFS(i);
+ reg = rzg2l_gpt_read(rzg2l_gpt, offs + RZG2L_GTST);
+ val |= FIELD_GET(RZG2L_GTST_POEG_IRQ_MASK, reg);
+ }
+
+ return val;
+}
+EXPORT_SYMBOL_GPL(rzg2l_gpt_poeg_disable_req_irq_status);
+
+int rzg2l_gpt_poeg_disable_req_clr(void *dev, u8 grp)
+{
+ u8 bitpos = grp * RZG2L_MAX_HW_CHANNELS;
+ struct rzg2l_gpt_chip *rzg2l_gpt;
+ unsigned int i;
+ u32 offs;
+ u32 reg;
+
+ rzg2l_gpt = dev_get_drvdata(dev);
+ for (i = 0; i < RZG2L_MAX_HW_CHANNELS; i++) {
+ if (!test_bit(bitpos + i, rzg2l_gpt->poeg_gpt_link))
+ continue;
+
+ offs = RZG2L_GET_CH_OFFS(i);
+ reg = rzg2l_gpt_read(rzg2l_gpt, offs + RZG2L_GTST);
+
+ if (reg & (RZG2L_GTST_OABHF | RZG2L_GTST_OABLF))
+ rzg2l_gpt_modify(rzg2l_gpt, offs + RZG2L_GTIOR,
+ RZG2L_GTIOR_OBE, 0);
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(rzg2l_gpt_poeg_disable_req_clr);
+
+int rzg2l_gpt_pin_reenable(void *dev, u8 grp)
+{
+ u8 bitpos = grp * RZG2L_MAX_HW_CHANNELS;
+ struct rzg2l_gpt_chip *rzg2l_gpt;
+ unsigned int i;
+ u32 offs;
+
+ rzg2l_gpt = dev_get_drvdata(dev);
+ for (i = 0; i < RZG2L_MAX_HW_CHANNELS; i++) {
+ if (!test_bit(bitpos + i, rzg2l_gpt->poeg_gpt_link))
+ continue;
+
+ offs = RZG2L_GET_CH_OFFS(i);
+ rzg2l_gpt_modify(rzg2l_gpt, offs + RZG2L_GTIOR,
+ RZG2L_GTIOR_OBE, RZG2L_GTIOR_OBE);
+ }
+ return 0;
+}
+EXPORT_SYMBOL_GPL(rzg2l_gpt_pin_reenable);
+
+static int rzg2l_gpt_poeg_disable_req_endisable(void *dev, u8 grp, int op, bool on)
+{
+ u8 bitpos = grp * RZG2L_MAX_HW_CHANNELS;
+ struct rzg2l_gpt_chip *rzg2l_gpt;
+ unsigned int i;
+ u32 offs;
+
+ rzg2l_gpt = dev_get_drvdata(dev);
+ pm_runtime_get_sync(dev);
+
+ for (i = 0; i < RZG2L_MAX_HW_CHANNELS; i++) {
+ if (!test_bit(bitpos + i, rzg2l_gpt->poeg_gpt_link))
+ continue;
+
+ offs = RZG2L_GET_CH_OFFS(i);
+ if (on)
+ rzg2l_gpt_modify(rzg2l_gpt, offs + RZG2L_GTINTAD, op, op);
+ else
+ rzg2l_gpt_modify(rzg2l_gpt, offs + RZG2L_GTINTAD, op, 0);
+ }
+
+ pm_runtime_put(dev);
+
+ return 0;
+}
+
+int rzg2l_gpt_poeg_disable_req_both_high(void *dev, u8 grp, bool on)
+{
+ int id = RZG2L_GTINTAD_OUTPUT_DISABLE_SAME_LEVEL_HIGH;
+
+ return rzg2l_gpt_poeg_disable_req_endisable(dev, grp, id, on);
+}
+EXPORT_SYMBOL_GPL(rzg2l_gpt_poeg_disable_req_both_high);
+
static void rzg2l_gpt_reset_assert_pm_disable(void *data)
{
struct rzg2l_gpt_chip *rzg2l_gpt = data;
diff --git a/include/linux/pwm/rzg2l-gpt.h b/include/linux/pwm/rzg2l-gpt.h
new file mode 100644
index 000000000000..0fc13ab57420
--- /dev/null
+++ b/include/linux/pwm/rzg2l-gpt.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __LINUX_PWM_RZG2L_GPT_H__
+#define __LINUX_PWM_RZG2L_GPT_H__
+
+#if IS_ENABLED(CONFIG_PWM_RZG2L_GPT)
+u32 rzg2l_gpt_poeg_disable_req_irq_status(void *dev, u8 grp);
+int rzg2l_gpt_poeg_disable_req_clr(void *gpt_device, u8 grp);
+int rzg2l_gpt_pin_reenable(void *gpt_device, u8 grp);
+int rzg2l_gpt_poeg_disable_req_both_high(void *gpt_device, u8 grp, bool on);
+#else
+static inline u32 rzg2l_gpt_poeg_disable_req_irq_status(void *dev, u8 grp)
+{
+ return -ENODEV;
+}
+
+static inline int rzg2l_gpt_poeg_disable_req_clr(void *gpt_device, u8 grp)
+{
+ return -ENODEV;
+}
+
+static inline int rzg2l_gpt_pin_reenable(void *gpt_device, u8 grp)
+{
+ return -ENODEV;
+}
+
+static inline int rzg2l_gpt_poeg_disable_req_both_high(void *gpt_device, u8 grp, bool on)
+{
+ return -ENODEV;
+}
+#endif
+
+#endif /* __LINUX_PWM_RZG2L_GPT_H__ */
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread* [DO NOT APPLY PATCH v6 08/13] pinctrl: renesas: rzg2l-poeg: Add support for GPT Output-Disable Request
2023-03-06 9:00 [PATCH v6 00/13] Add pinctrl sysfs and RZ/G2L POEG support Biju Das
` (5 preceding siblings ...)
2023-03-06 9:00 ` [DO NOT APPLY PATCH v6 07/13] pwm: rzg2l-gpt: Add support for output disable request from gpt Biju Das
@ 2023-03-06 9:00 ` Biju Das
2023-03-06 9:00 ` [DO NOT APPLY PATCH v6 09/13] pwm: rzg2l-gpt: Add support for output disable when both output low Biju Das
` (4 subsequent siblings)
11 siblings, 0 replies; 44+ messages in thread
From: Biju Das @ 2023-03-06 9:00 UTC (permalink / raw)
To: Linus Walleij, Thierry Reding
Cc: Biju Das, Uwe Kleine-König, Geert Uytterhoeven, Magnus Damm,
linux-pwm, linux-renesas-soc, linux-gpio, Prabhakar Mahadev Lad
This patch supports output-disable requests from GPT.
Added sysfs to enable/disable request from GPT when both outputs
are high.
When both outputs are high, gpt detects the condition and triggers
an interrupt to POEG. POEG handles the interrupt and send notification
to userspace. userspace handles the fault and issue a write call to
cancel the disable output request.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
drivers/pinctrl/renesas/poeg/rzg2l-poeg.c | 206 +++++++++++++++++++++-
include/linux/pinctrl/pinctrl-rzg2l.h | 9 +
2 files changed, 212 insertions(+), 3 deletions(-)
diff --git a/drivers/pinctrl/renesas/poeg/rzg2l-poeg.c b/drivers/pinctrl/renesas/poeg/rzg2l-poeg.c
index 30f4352e257d..b6f01065c058 100644
--- a/drivers/pinctrl/renesas/poeg/rzg2l-poeg.c
+++ b/drivers/pinctrl/renesas/poeg/rzg2l-poeg.c
@@ -4,27 +4,45 @@
*
* Copyright (C) 2022 Renesas Electronics Corporation
*/
+#include <linux/cdev.h>
+#include <linux/interrupt.h>
#include <linux/io.h>
+#include <linux/kfifo.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_platform.h>
#include <linux/pinctrl/pinctrl-rzg2l.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
+#include <linux/pwm/rzg2l-gpt.h>
+#include <linux/poll.h>
#include <linux/reset.h>
+#include <linux/wait.h>
+#define POEGG_IOCE BIT(5)
+#define POEGG_PIDE BIT(4)
#define POEGG_SSF BIT(3)
+#define POEGG_IOCF BIT(1)
+#define POEGG_PIDF BIT(0)
#define RZG2L_POEG_MAX_INDEX 3
#define RZG2L_GPT_MAX_HW_CHANNELS 8
#define RZG2L_GPT_INVALID_CHANNEL 0xff
+static struct class *poeg_class;
+static dev_t g_poeg_dev;
+static int minor_n;
+
struct rzg2l_poeg_chip {
struct device *gpt_dev;
struct reset_control *rstc;
void __iomem *mmio;
u8 index;
+ DECLARE_BITMAP(gpt_irq, 3);
+ struct cdev poeg_cdev;
+ wait_queue_head_t events_wait;
+ DECLARE_KFIFO_PTR(events, struct poeg_event);
u8 gpt_channels[RZG2L_GPT_MAX_HW_CHANNELS];
};
@@ -65,6 +83,20 @@ static int rzg2l_poeg_output_disable_user(struct rzg2l_poeg_chip *chip,
return 0;
}
+static int rzg2l_poeg_output_disable_both_high(struct rzg2l_poeg_chip *chip,
+ bool enable)
+{
+ if (enable)
+ set_bit(RZG2L_GPT_OABHF, chip->gpt_irq);
+ else
+ clear_bit(RZG2L_GPT_OABHF, chip->gpt_irq);
+
+ rzg2l_gpt_poeg_disable_req_both_high(chip->gpt_dev, chip->index,
+ test_bit(RZG2L_GPT_OABHF, chip->gpt_irq));
+
+ return 0;
+}
+
static int rzg2l_poeg_cb(void *context, const char *fname, const char *gname,
enum pin_output_disable_conf conf,
unsigned int conf_val)
@@ -88,6 +120,8 @@ static int rzg2l_poeg_cb(void *context, const char *fname, const char *gname,
ret = rzg2l_poeg_output_disable_user(context, !!conf_val);
break;
case PINCTRL_OUTPUT_DISABLE_BY_SOC_ON_PIN_OUTPUT_HIGH:
+ ret = rzg2l_poeg_output_disable_both_high(context, !!conf_val);
+ break;
case PINCTRL_OUTPUT_DISABLE_BY_SOC_ON_PIN_OUTPUT_LOW:
case PINCTRL_OUTPUT_DISABLE_BY_SOC_ON_DEAD_TIME_ERROR:
default:
@@ -97,6 +131,111 @@ static int rzg2l_poeg_cb(void *context, const char *fname, const char *gname,
return ret;
}
+static irqreturn_t rzg2l_poeg_irq(int irq, void *ptr)
+{
+ struct rzg2l_poeg_chip *chip = ptr;
+ struct poeg_event ev;
+ u32 val;
+
+ val = rzg2l_gpt_poeg_disable_req_irq_status(chip->gpt_dev, chip->index);
+ ev.channel = chip->index;
+ ev.gpt_disable_irq_status = val;
+ kfifo_in(&chip->events, &ev, 1);
+ wake_up_poll(&chip->events_wait, EPOLLIN);
+
+ val = rzg2l_poeg_read(chip);
+ if (val & POEGG_IOCF)
+ val &= ~POEGG_IOCF;
+
+ if (val & POEGG_PIDF)
+ val &= ~POEGG_PIDF;
+
+ rzg2l_poeg_write(chip, val);
+ rzg2l_gpt_poeg_disable_req_clr(chip->gpt_dev, chip->index);
+
+ return IRQ_HANDLED;
+}
+
+static __poll_t rzg2l_poeg_chrdev_poll(struct file *filp,
+ struct poll_table_struct *pollt)
+{
+ struct rzg2l_poeg_chip *const chip = filp->private_data;
+ __poll_t events = 0;
+
+ poll_wait(filp, &chip->events_wait, pollt);
+ if (!kfifo_is_empty(&chip->events))
+ events = EPOLLIN | EPOLLRDNORM;
+
+ return events;
+}
+
+static ssize_t rzg2l_poeg_chrdev_read(struct file *filp, char __user *buf,
+ size_t len, loff_t *f_ps)
+{
+ struct rzg2l_poeg_chip *const chip = filp->private_data;
+ unsigned int copied;
+ int err;
+
+ if (len < sizeof(struct poeg_event))
+ return -EINVAL;
+
+ do {
+ if (kfifo_is_empty(&chip->events)) {
+ if (filp->f_flags & O_NONBLOCK)
+ return -EAGAIN;
+
+ err = wait_event_interruptible(chip->events_wait,
+ !kfifo_is_empty(&chip->events));
+ if (err < 0)
+ return err;
+ }
+
+ err = kfifo_to_user(&chip->events, buf, len, &copied);
+ if (err < 0)
+ return err;
+ } while (!copied);
+
+ return copied;
+}
+
+static ssize_t rzg2l_poeg_chrdev_write(struct file *filp,
+ const char __user *buf,
+ size_t len, loff_t *f_ps)
+{
+ struct rzg2l_poeg_chip *const chip = filp->private_data;
+
+ rzg2l_gpt_pin_reenable(chip->gpt_dev, chip->index);
+
+ return len;
+}
+
+static int rzg2l_poeg_chrdev_open(struct inode *inode, struct file *filp)
+{
+ struct rzg2l_poeg_chip *const chip = container_of(inode->i_cdev,
+ typeof(*chip),
+ poeg_cdev);
+
+ filp->private_data = chip;
+
+ return nonseekable_open(inode, filp);
+}
+
+static int rzg2l_poeg_chrdev_release(struct inode *inode, struct file *filp)
+{
+ filp->private_data = NULL;
+
+ return 0;
+}
+
+static const struct file_operations poeg_fops = {
+ .owner = THIS_MODULE,
+ .read = rzg2l_poeg_chrdev_read,
+ .write = rzg2l_poeg_chrdev_write,
+ .poll = rzg2l_poeg_chrdev_poll,
+ .open = rzg2l_poeg_chrdev_open,
+ .release = rzg2l_poeg_chrdev_release,
+};
+
static bool rzg2l_poeg_get_linked_gpt_channels(struct platform_device *pdev,
struct rzg2l_poeg_chip *chip,
struct device_node *gpt_np,
@@ -168,6 +307,7 @@ static int rzg2l_poeg_probe(struct platform_device *pdev)
struct device_node *np;
u32 val;
int ret;
+ int irq;
chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
if (!chip)
@@ -199,10 +339,20 @@ static int rzg2l_poeg_probe(struct platform_device *pdev)
if (IS_ERR(chip->mmio))
return PTR_ERR(chip->mmio);
- if (gpt_linked)
+ if (gpt_linked) {
rzg2l_output_disable_cb_register(chip->gpt_dev,
rzg2l_poeg_cb, chip);
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0)
+ return irq;
+
+ ret = devm_request_irq(&pdev->dev, irq, rzg2l_poeg_irq, 0,
+ dev_name(&pdev->dev), chip);
+ if (ret < 0)
+ return dev_err_probe(&pdev->dev, ret, "cannot get irq\n");
+ }
+
chip->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
if (IS_ERR(chip->rstc))
return dev_err_probe(&pdev->dev, PTR_ERR(chip->rstc),
@@ -220,8 +370,24 @@ static int rzg2l_poeg_probe(struct platform_device *pdev)
goto err_pm_disable;
}
- return 0;
+ if (gpt_linked)
+ rzg2l_poeg_write(chip, POEGG_IOCE | POEGG_PIDE);
+
+ init_waitqueue_head(&chip->events_wait);
+ cdev_init(&chip->poeg_cdev, &poeg_fops);
+ chip->poeg_cdev.owner = THIS_MODULE;
+ ret = cdev_add(&chip->poeg_cdev, MKDEV(MAJOR(g_poeg_dev), minor_n), 1);
+ if (ret)
+ goto err_pm;
+
+ device_create(poeg_class, NULL, MKDEV(MAJOR(g_poeg_dev), minor_n),
+ NULL, "poeg%d", minor_n);
+ minor_n++;
+
+ return kfifo_alloc(&chip->events, 64, GFP_KERNEL);
+err_pm:
+ pm_runtime_put(&pdev->dev);
err_pm_disable:
pm_runtime_disable(&pdev->dev);
reset_control_assert(chip->rstc);
@@ -232,6 +398,10 @@ static int rzg2l_poeg_remove(struct platform_device *pdev)
{
struct rzg2l_poeg_chip *chip = platform_get_drvdata(pdev);
+ kfifo_free(&chip->events);
+ device_destroy(poeg_class,
+ MKDEV(MAJOR(g_poeg_dev), MINOR(chip->poeg_cdev.dev)));
+ cdev_del(&chip->poeg_cdev);
pm_runtime_put(&pdev->dev);
pm_runtime_disable(&pdev->dev);
reset_control_assert(chip->rstc);
@@ -247,7 +417,37 @@ static struct platform_driver rzg2l_poeg_driver = {
.probe = rzg2l_poeg_probe,
.remove = rzg2l_poeg_remove,
};
-module_platform_driver(rzg2l_poeg_driver);
+
+static int rzg2l_poeg_device_init(void)
+{
+ int err;
+
+ err = alloc_chrdev_region(&g_poeg_dev, 0, 1, "poeg");
+ if (err)
+ goto out;
+
+ poeg_class = class_create(THIS_MODULE, "poeg");
+ if (IS_ERR(poeg_class)) {
+ err = PTR_ERR(poeg_class);
+ goto err_free_chrdev;
+ }
+
+ return platform_driver_register(&rzg2l_poeg_driver);
+
+err_free_chrdev:
+ unregister_chrdev_region(g_poeg_dev, 1);
+out:
+ return err;
+}
+
+static void rzg2l_poeg_device_exit(void)
+{
+ class_destroy(poeg_class);
+ unregister_chrdev_region(g_poeg_dev, 1);
+}
+
+module_init(rzg2l_poeg_device_init);
+module_exit(rzg2l_poeg_device_exit);
MODULE_AUTHOR("Biju Das <biju.das.jz@bp.renesas.com>");
MODULE_DESCRIPTION("Renesas RZ/G2L POEG Driver");
diff --git a/include/linux/pinctrl/pinctrl-rzg2l.h b/include/linux/pinctrl/pinctrl-rzg2l.h
index a49b4c5f8908..94d1b12d84c8 100644
--- a/include/linux/pinctrl/pinctrl-rzg2l.h
+++ b/include/linux/pinctrl/pinctrl-rzg2l.h
@@ -4,6 +4,15 @@
#include <linux/pinctrl/output-disable.h>
+#define RZG2L_GPT_DTEF 0
+#define RZG2L_GPT_OABHF 1
+#define RZG2L_GPT_OABLF 2
+
+struct poeg_event {
+ __u32 gpt_disable_irq_status;
+ __u8 channel;
+};
+
typedef int (*output_disable_cb) (void *context,
const char *fname,
const char *gname,
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread* [DO NOT APPLY PATCH v6 09/13] pwm: rzg2l-gpt: Add support for output disable when both output low
2023-03-06 9:00 [PATCH v6 00/13] Add pinctrl sysfs and RZ/G2L POEG support Biju Das
` (6 preceding siblings ...)
2023-03-06 9:00 ` [DO NOT APPLY PATCH v6 08/13] pinctrl: renesas: rzg2l-poeg: Add support for GPT Output-Disable Request Biju Das
@ 2023-03-06 9:00 ` Biju Das
2023-03-06 9:00 ` [DO NOT APPLY PATCH v6 10/13] pinctrl: renesas: rzg2l-poeg: output-disable request from GPT when both outputs are low Biju Das
` (3 subsequent siblings)
11 siblings, 0 replies; 44+ messages in thread
From: Biju Das @ 2023-03-06 9:00 UTC (permalink / raw)
To: Linus Walleij, Thierry Reding
Cc: Biju Das, Uwe Kleine-König, Geert Uytterhoeven, Magnus Damm,
linux-pwm, linux-renesas-soc, linux-gpio, Prabhakar Mahadev Lad
This patch adds support for output disable request from gpt,
when same time output level is low.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
drivers/pwm/pwm-rzg2l-gpt.c | 9 +++++++++
include/linux/pwm/rzg2l-gpt.h | 6 ++++++
2 files changed, 15 insertions(+)
diff --git a/drivers/pwm/pwm-rzg2l-gpt.c b/drivers/pwm/pwm-rzg2l-gpt.c
index 2f138e95f752..2291cc3cff39 100644
--- a/drivers/pwm/pwm-rzg2l-gpt.c
+++ b/drivers/pwm/pwm-rzg2l-gpt.c
@@ -75,6 +75,7 @@
#define RZG2L_GTINTAD_GRP_MASK GENMASK(25, 24)
#define RZG2L_GTINTAD_OUTPUT_DISABLE_SAME_LEVEL_HIGH BIT(29)
+#define RZG2L_GTINTAD_OUTPUT_DISABLE_SAME_LEVEL_LOW BIT(30)
#define RZG2L_GTST_OABHF BIT(29)
#define RZG2L_GTST_OABLF BIT(30)
@@ -542,6 +543,14 @@ int rzg2l_gpt_poeg_disable_req_both_high(void *dev, u8 grp, bool on)
}
EXPORT_SYMBOL_GPL(rzg2l_gpt_poeg_disable_req_both_high);
+int rzg2l_gpt_poeg_disable_req_both_low(void *dev, u8 grp, bool on)
+{
+ int id = RZG2L_GTINTAD_OUTPUT_DISABLE_SAME_LEVEL_LOW;
+
+ return rzg2l_gpt_poeg_disable_req_endisable(dev, grp, id, on);
+}
+EXPORT_SYMBOL_GPL(rzg2l_gpt_poeg_disable_req_both_low);
+
static void rzg2l_gpt_reset_assert_pm_disable(void *data)
{
struct rzg2l_gpt_chip *rzg2l_gpt = data;
diff --git a/include/linux/pwm/rzg2l-gpt.h b/include/linux/pwm/rzg2l-gpt.h
index 0fc13ab57420..592bc2900c9e 100644
--- a/include/linux/pwm/rzg2l-gpt.h
+++ b/include/linux/pwm/rzg2l-gpt.h
@@ -7,6 +7,7 @@ u32 rzg2l_gpt_poeg_disable_req_irq_status(void *dev, u8 grp);
int rzg2l_gpt_poeg_disable_req_clr(void *gpt_device, u8 grp);
int rzg2l_gpt_pin_reenable(void *gpt_device, u8 grp);
int rzg2l_gpt_poeg_disable_req_both_high(void *gpt_device, u8 grp, bool on);
+int rzg2l_gpt_poeg_disable_req_both_low(void *gpt_device, u8 grp, bool on);
#else
static inline u32 rzg2l_gpt_poeg_disable_req_irq_status(void *dev, u8 grp)
{
@@ -27,6 +28,11 @@ static inline int rzg2l_gpt_poeg_disable_req_both_high(void *gpt_device, u8 grp,
{
return -ENODEV;
}
+
+static inline int rzg2l_gpt_poeg_disable_req_both_low(void *gpt_device, u8 grp, bool on)
+{
+ return -ENODEV;
+}
#endif
#endif /* __LINUX_PWM_RZG2L_GPT_H__ */
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread* [DO NOT APPLY PATCH v6 10/13] pinctrl: renesas: rzg2l-poeg: output-disable request from GPT when both outputs are low.
2023-03-06 9:00 [PATCH v6 00/13] Add pinctrl sysfs and RZ/G2L POEG support Biju Das
` (7 preceding siblings ...)
2023-03-06 9:00 ` [DO NOT APPLY PATCH v6 09/13] pwm: rzg2l-gpt: Add support for output disable when both output low Biju Das
@ 2023-03-06 9:00 ` Biju Das
2023-03-06 23:39 ` andy.shevchenko
2023-03-06 9:00 ` [DO NOT APPLY PATCH v6 11/13] pwm: rzg2l-gpt: Add support for output disable on dead time error Biju Das
` (2 subsequent siblings)
11 siblings, 1 reply; 44+ messages in thread
From: Biju Das @ 2023-03-06 9:00 UTC (permalink / raw)
To: Linus Walleij, Thierry Reding
Cc: Biju Das, Uwe Kleine-König, Geert Uytterhoeven, Magnus Damm,
linux-pwm, linux-renesas-soc, linux-gpio, Prabhakar Mahadev Lad
This patch adds support fpr output-disable requests from GPT, when both
outputs are low.
Added sysfs to enable/disable for configuring GPT output disable request
when both outputs are low.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
drivers/pinctrl/renesas/poeg/rzg2l-poeg.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/pinctrl/renesas/poeg/rzg2l-poeg.c b/drivers/pinctrl/renesas/poeg/rzg2l-poeg.c
index b6f01065c058..bbca21557a70 100644
--- a/drivers/pinctrl/renesas/poeg/rzg2l-poeg.c
+++ b/drivers/pinctrl/renesas/poeg/rzg2l-poeg.c
@@ -97,6 +97,20 @@ static int rzg2l_poeg_output_disable_both_high(struct rzg2l_poeg_chip *chip,
return 0;
}
+static int rzg2l_poeg_output_disable_both_low(struct rzg2l_poeg_chip *chip,
+ bool enable)
+{
+ if (enable)
+ set_bit(RZG2L_GPT_OABLF, chip->gpt_irq);
+ else
+ clear_bit(RZG2L_GPT_OABLF, chip->gpt_irq);
+
+ rzg2l_gpt_poeg_disable_req_both_low(chip->gpt_dev, chip->index,
+ test_bit(RZG2L_GPT_OABLF, chip->gpt_irq));
+
+ return 0;
+}
+
static int rzg2l_poeg_cb(void *context, const char *fname, const char *gname,
enum pin_output_disable_conf conf,
unsigned int conf_val)
@@ -123,6 +137,8 @@ static int rzg2l_poeg_cb(void *context, const char *fname, const char *gname,
ret = rzg2l_poeg_output_disable_both_high(context, !!conf_val);
break;
case PINCTRL_OUTPUT_DISABLE_BY_SOC_ON_PIN_OUTPUT_LOW:
+ ret = rzg2l_poeg_output_disable_both_low(context, !!conf_val);
+ break;
case PINCTRL_OUTPUT_DISABLE_BY_SOC_ON_DEAD_TIME_ERROR:
default:
return -EINVAL;
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [DO NOT APPLY PATCH v6 10/13] pinctrl: renesas: rzg2l-poeg: output-disable request from GPT when both outputs are low.
2023-03-06 9:00 ` [DO NOT APPLY PATCH v6 10/13] pinctrl: renesas: rzg2l-poeg: output-disable request from GPT when both outputs are low Biju Das
@ 2023-03-06 23:39 ` andy.shevchenko
2023-03-07 8:57 ` Biju Das
0 siblings, 1 reply; 44+ messages in thread
From: andy.shevchenko @ 2023-03-06 23:39 UTC (permalink / raw)
To: Biju Das
Cc: Linus Walleij, Thierry Reding, Uwe Kleine-König,
Geert Uytterhoeven, Magnus Damm, linux-pwm, linux-renesas-soc,
linux-gpio, Prabhakar Mahadev Lad
Mon, Mar 06, 2023 at 09:00:11AM +0000, Biju Das kirjoitti:
> This patch adds support fpr output-disable requests from GPT, when both
> outputs are low.
>
> Added sysfs to enable/disable for configuring GPT output disable request
> when both outputs are low.
...
> +static int rzg2l_poeg_output_disable_both_low(struct rzg2l_poeg_chip *chip,
> + bool enable)
> +{
> + if (enable)
> + set_bit(RZG2L_GPT_OABLF, chip->gpt_irq);
> + else
> + clear_bit(RZG2L_GPT_OABLF, chip->gpt_irq);
JFYI: assign_bit()
> + rzg2l_gpt_poeg_disable_req_both_low(chip->gpt_dev, chip->index,
> + test_bit(RZG2L_GPT_OABLF, chip->gpt_irq));
> +
> + return 0;
> +}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 44+ messages in thread* RE: [DO NOT APPLY PATCH v6 10/13] pinctrl: renesas: rzg2l-poeg: output-disable request from GPT when both outputs are low.
2023-03-06 23:39 ` andy.shevchenko
@ 2023-03-07 8:57 ` Biju Das
0 siblings, 0 replies; 44+ messages in thread
From: Biju Das @ 2023-03-07 8:57 UTC (permalink / raw)
To: andy.shevchenko@gmail.com
Cc: Linus Walleij, Thierry Reding, Uwe Kleine-König,
Geert Uytterhoeven, Magnus Damm, linux-pwm@vger.kernel.org,
linux-renesas-soc@vger.kernel.org, linux-gpio@vger.kernel.org,
Prabhakar Mahadev Lad
Hi Andy Shevchenko,
Thanks for the feedback.
> Subject: Re: [DO NOT APPLY PATCH v6 10/13] pinctrl: renesas: rzg2l-poeg:
> output-disable request from GPT when both outputs are low.
>
> Mon, Mar 06, 2023 at 09:00:11AM +0000, Biju Das kirjoitti:
> > This patch adds support fpr output-disable requests from GPT, when
> > both outputs are low.
> >
> > Added sysfs to enable/disable for configuring GPT output disable
> > request when both outputs are low.
>
> ...
>
> > +static int rzg2l_poeg_output_disable_both_low(struct rzg2l_poeg_chip
> *chip,
> > + bool enable)
> > +{
> > + if (enable)
> > + set_bit(RZG2L_GPT_OABLF, chip->gpt_irq);
> > + else
> > + clear_bit(RZG2L_GPT_OABLF, chip->gpt_irq);
>
> JFYI: assign_bit()
OK, will use assign_bit() and remove the above code.
Cheers,
Biju
>
>
> > + rzg2l_gpt_poeg_disable_req_both_low(chip->gpt_dev, chip->index,
> > + test_bit(RZG2L_GPT_OABLF, chip-
> >gpt_irq));
> > +
> > + return 0;
> > +}
>
> --
> With Best Regards,
> Andy Shevchenko
>
^ permalink raw reply [flat|nested] 44+ messages in thread
* [DO NOT APPLY PATCH v6 11/13] pwm: rzg2l-gpt: Add support for output disable on dead time error
2023-03-06 9:00 [PATCH v6 00/13] Add pinctrl sysfs and RZ/G2L POEG support Biju Das
` (8 preceding siblings ...)
2023-03-06 9:00 ` [DO NOT APPLY PATCH v6 10/13] pinctrl: renesas: rzg2l-poeg: output-disable request from GPT when both outputs are low Biju Das
@ 2023-03-06 9:00 ` Biju Das
2023-03-06 9:00 ` [DO NOT APPLY PATCH v6 12/13] pinctrl: renesas: rzg2l-poeg: output-disable request from GPT " Biju Das
2023-03-06 9:00 ` [DO NOT APPLY PATCH v6 13/13] tools/poeg: Add test app for poeg Biju Das
11 siblings, 0 replies; 44+ messages in thread
From: Biju Das @ 2023-03-06 9:00 UTC (permalink / raw)
To: Linus Walleij, Thierry Reding
Cc: Biju Das, Uwe Kleine-König, Geert Uytterhoeven, Magnus Damm,
linux-pwm, linux-renesas-soc, linux-gpio, Prabhakar Mahadev Lad
This patch adds support for output disable request from gpt,
when dead time error occurred.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
drivers/pwm/pwm-rzg2l-gpt.c | 9 +++++++++
include/linux/pwm/rzg2l-gpt.h | 6 ++++++
2 files changed, 15 insertions(+)
diff --git a/drivers/pwm/pwm-rzg2l-gpt.c b/drivers/pwm/pwm-rzg2l-gpt.c
index 2291cc3cff39..c88a5bf9e31d 100644
--- a/drivers/pwm/pwm-rzg2l-gpt.c
+++ b/drivers/pwm/pwm-rzg2l-gpt.c
@@ -74,6 +74,7 @@
(FIELD_PREP(RZG2L_GTIOR_GTIOB, RZG2L_INIT_OUT_LO_OUT_LO_END_TOGGLE) | RZG2L_GTIOR_OBE)
#define RZG2L_GTINTAD_GRP_MASK GENMASK(25, 24)
+#define RZG2L_GTINTAD_OUTPUT_DISABLE_DEADTIME_ERROR BIT(28)
#define RZG2L_GTINTAD_OUTPUT_DISABLE_SAME_LEVEL_HIGH BIT(29)
#define RZG2L_GTINTAD_OUTPUT_DISABLE_SAME_LEVEL_LOW BIT(30)
@@ -551,6 +552,14 @@ int rzg2l_gpt_poeg_disable_req_both_low(void *dev, u8 grp, bool on)
}
EXPORT_SYMBOL_GPL(rzg2l_gpt_poeg_disable_req_both_low);
+int rzg2l_gpt_poeg_disable_req_deadtime_error(void *dev, u8 grp, bool on)
+{
+ int id = RZG2L_GTINTAD_OUTPUT_DISABLE_DEADTIME_ERROR;
+
+ return rzg2l_gpt_poeg_disable_req_endisable(dev, grp, id, on);
+}
+EXPORT_SYMBOL_GPL(rzg2l_gpt_poeg_disable_req_deadtime_error);
+
static void rzg2l_gpt_reset_assert_pm_disable(void *data)
{
struct rzg2l_gpt_chip *rzg2l_gpt = data;
diff --git a/include/linux/pwm/rzg2l-gpt.h b/include/linux/pwm/rzg2l-gpt.h
index 592bc2900c9e..8a004c690c2a 100644
--- a/include/linux/pwm/rzg2l-gpt.h
+++ b/include/linux/pwm/rzg2l-gpt.h
@@ -8,6 +8,7 @@ int rzg2l_gpt_poeg_disable_req_clr(void *gpt_device, u8 grp);
int rzg2l_gpt_pin_reenable(void *gpt_device, u8 grp);
int rzg2l_gpt_poeg_disable_req_both_high(void *gpt_device, u8 grp, bool on);
int rzg2l_gpt_poeg_disable_req_both_low(void *gpt_device, u8 grp, bool on);
+int rzg2l_gpt_poeg_disable_req_deadtime_error(void *gpt_device, u8 grp, bool on);
#else
static inline u32 rzg2l_gpt_poeg_disable_req_irq_status(void *dev, u8 grp)
{
@@ -33,6 +34,11 @@ static inline int rzg2l_gpt_poeg_disable_req_both_low(void *gpt_device, u8 grp,
{
return -ENODEV;
}
+
+static inline int rzg2l_gpt_poeg_disable_req_deadtime_err(void *gpt_device, u8 grp, bool on)
+{
+ return -ENODEV;
+}
#endif
#endif /* __LINUX_PWM_RZG2L_GPT_H__ */
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread* [DO NOT APPLY PATCH v6 12/13] pinctrl: renesas: rzg2l-poeg: output-disable request from GPT on dead time error
2023-03-06 9:00 [PATCH v6 00/13] Add pinctrl sysfs and RZ/G2L POEG support Biju Das
` (9 preceding siblings ...)
2023-03-06 9:00 ` [DO NOT APPLY PATCH v6 11/13] pwm: rzg2l-gpt: Add support for output disable on dead time error Biju Das
@ 2023-03-06 9:00 ` Biju Das
2023-03-06 9:00 ` [DO NOT APPLY PATCH v6 13/13] tools/poeg: Add test app for poeg Biju Das
11 siblings, 0 replies; 44+ messages in thread
From: Biju Das @ 2023-03-06 9:00 UTC (permalink / raw)
To: Linus Walleij, Thierry Reding
Cc: Biju Das, Uwe Kleine-König, Geert Uytterhoeven, Magnus Damm,
linux-pwm, linux-renesas-soc, linux-gpio, Prabhakar Mahadev Lad
This patch adds support for output-disable requests from GPT,
when dead time error occurs.
Added sysfs to enable/disable for configuring GPT output disable request
for dead time error.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
drivers/pinctrl/renesas/poeg/rzg2l-poeg.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/pinctrl/renesas/poeg/rzg2l-poeg.c b/drivers/pinctrl/renesas/poeg/rzg2l-poeg.c
index bbca21557a70..3d0801d8937a 100644
--- a/drivers/pinctrl/renesas/poeg/rzg2l-poeg.c
+++ b/drivers/pinctrl/renesas/poeg/rzg2l-poeg.c
@@ -111,6 +111,20 @@ static int rzg2l_poeg_output_disable_both_low(struct rzg2l_poeg_chip *chip,
return 0;
}
+static int rzg2l_poeg_output_disable_deadtime_err(struct rzg2l_poeg_chip *chip,
+ bool enable)
+{
+ if (enable)
+ set_bit(RZG2L_GPT_DTEF, chip->gpt_irq);
+ else
+ clear_bit(RZG2L_GPT_DTEF, chip->gpt_irq);
+
+ rzg2l_gpt_poeg_disable_req_deadtime_error(chip->gpt_dev, chip->index,
+ test_bit(RZG2L_GPT_DTEF, chip->gpt_irq));
+
+ return 0;
+}
+
static int rzg2l_poeg_cb(void *context, const char *fname, const char *gname,
enum pin_output_disable_conf conf,
unsigned int conf_val)
@@ -140,6 +154,8 @@ static int rzg2l_poeg_cb(void *context, const char *fname, const char *gname,
ret = rzg2l_poeg_output_disable_both_low(context, !!conf_val);
break;
case PINCTRL_OUTPUT_DISABLE_BY_SOC_ON_DEAD_TIME_ERROR:
+ ret = rzg2l_poeg_output_disable_deadtime_err(context, !!conf_val);
+ break;
default:
return -EINVAL;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread* [DO NOT APPLY PATCH v6 13/13] tools/poeg: Add test app for poeg
2023-03-06 9:00 [PATCH v6 00/13] Add pinctrl sysfs and RZ/G2L POEG support Biju Das
` (10 preceding siblings ...)
2023-03-06 9:00 ` [DO NOT APPLY PATCH v6 12/13] pinctrl: renesas: rzg2l-poeg: output-disable request from GPT " Biju Das
@ 2023-03-06 9:00 ` Biju Das
11 siblings, 0 replies; 44+ messages in thread
From: Biju Das @ 2023-03-06 9:00 UTC (permalink / raw)
To: Linus Walleij, Thierry Reding
Cc: Biju Das, Uwe Kleine-König, Geert Uytterhoeven, Magnus Damm,
linux-pwm, linux-renesas-soc, linux-gpio, Prabhakar Mahadev Lad
Add test app for poeg
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
tools/poeg/Build | 1 +
tools/poeg/Makefile | 53 ++++++++++++++++++++++++++++++++++++++
tools/poeg/poeg_app.c | 60 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 114 insertions(+)
create mode 100644 tools/poeg/Build
create mode 100644 tools/poeg/Makefile
create mode 100644 tools/poeg/poeg_app.c
diff --git a/tools/poeg/Build b/tools/poeg/Build
new file mode 100644
index 000000000000..f960920a4afb
--- /dev/null
+++ b/tools/poeg/Build
@@ -0,0 +1 @@
+poeg_app-y += poeg_app.o
diff --git a/tools/poeg/Makefile b/tools/poeg/Makefile
new file mode 100644
index 000000000000..6946e6956215
--- /dev/null
+++ b/tools/poeg/Makefile
@@ -0,0 +1,53 @@
+# SPDX-License-Identifier: GPL-2.0
+include ../scripts/Makefile.include
+
+bindir ?= /usr/bin
+
+ifeq ($(srctree),)
+srctree := $(patsubst %/,%,$(dir $(CURDIR)))
+srctree := $(patsubst %/,%,$(dir $(srctree)))
+endif
+
+# Do not use make's built-in rules
+# (this improves performance and avoids hard-to-debug behaviour);
+MAKEFLAGS += -r
+
+override CFLAGS += -O2 -Wall -g -D_GNU_SOURCE -I$(OUTPUT)include
+
+ALL_TARGETS := poeg_app
+ALL_PROGRAMS := $(patsubst %,$(OUTPUT)%,$(ALL_TARGETS))
+
+all: $(ALL_PROGRAMS)
+
+export srctree OUTPUT CC LD CFLAGS
+include $(srctree)/tools/build/Makefile.include
+
+#
+# We need the following to be outside of kernel tree
+#
+$(OUTPUT)include/linux/poeg.h: ../../include/linux/soc/renesas/rzg2l-poeg.h
+ mkdir -p $(OUTPUT)include/linux 2>&1 || true
+ ln -sf $(CURDIR)/../../include/linux/soc/renesas/rzg2l-poeg.h $@
+
+prepare: $(OUTPUT)include/linux/poeg.h
+
+COUNTER_EXAMPLE := $(OUTPUT)poeg_app.o
+$(COUNTER_EXAMPLE): prepare FORCE
+ $(Q)$(MAKE) $(build)=poeg_app
+$(OUTPUT)poeg_app: $(COUNTER_EXAMPLE)
+ $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) $< -o $@
+
+clean:
+ rm -f $(ALL_PROGRAMS)
+ rm -rf $(OUTPUT)include/linux/counter.h
+ find $(or $(OUTPUT),.) -name '*.o' -delete -o -name '\.*.d' -delete
+
+install: $(ALL_PROGRAMS)
+ install -d -m 755 $(DESTDIR)$(bindir); \
+ for program in $(ALL_PROGRAMS); do \
+ install $$program $(DESTDIR)$(bindir); \
+ done
+
+FORCE:
+
+.PHONY: all install clean FORCE prepare
diff --git a/tools/poeg/poeg_app.c b/tools/poeg/poeg_app.c
new file mode 100644
index 000000000000..79cacb8c60c5
--- /dev/null
+++ b/tools/poeg/poeg_app.c
@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * POEG - example userspace application
+ * Copyright (C) 2022 Biju Das
+ */
+#include <stdio.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <errno.h>
+#include <sys/ioctl.h>
+#include <linux/ioctl.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <sys/time.h>
+#include <sys/types.h>
+#include <poll.h>
+
+#include <linux/poeg.h>
+
+int main(int argc, char *arg[])
+{
+ struct poeg_event event_data;
+ unsigned int val;
+ int ret, fd, i;
+
+ fd = open("/dev/poeg3", O_RDWR);
+ if (fd < 0)
+ perror("open");
+ else
+ printf("[POEG]open\n");
+
+ for (;;) {
+ ret = read(fd, &event_data, sizeof(event_data));
+ if (ret == -1) {
+ perror("Failed to read event data");
+ return 1;
+ }
+
+ val = event_data.gpt_disable_irq_status;
+ if (val) {
+ /* emulate fault clearing condition by adding delay */
+ sleep(2);
+ for (i = 0; i < 8; i++) {
+ if (val & 7) {
+ printf("gpt ch:%u, irq=%x\n", i, val & 7);
+ ret = write(fd, &event_data, sizeof(event_data));
+ }
+ val >>= 3;
+ }
+ }
+ }
+
+ if (close(fd) != 0)
+ perror("close");
+ else
+ printf("[POEG]close\n");
+
+ return 0;
+}
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread