* Re: [PATCH 1/2] pinctrl: imx: Support building SCU pinctrl driver as module
From: Daniel Baluta @ 2020-07-16 15:58 UTC (permalink / raw)
To: Anson Huang, Aisheng Dong, festevam@gmail.com,
shawnguo@kernel.org, stefan@agner.ch, kernel@pengutronix.de,
linus.walleij@linaro.org, s.hauer@pengutronix.de,
linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
Cc: dl-linux-imx
In-Reply-To: <DB3PR0402MB3916C9FE00C0F4FC62ACB711F57F0@DB3PR0402MB3916.eurprd04.prod.outlook.com>
On 7/16/20 6:21 PM, Anson Huang wrote:
> Hi, Daniel
>
>
>> Subject: Re: [PATCH 1/2] pinctrl: imx: Support building SCU pinctrl driver as
>> module
>>
>> Hi Anson,
>>
>> Few comments inline:
>>
>> On 7/16/20 6:06 PM, Anson Huang wrote:
>>> To support building i.MX SCU pinctrl driver as module, below things need to
>> be changed:
>>> - Export SCU related functions and use "IS_ENABLED" instead of
>>> "ifdef" to support SCU pinctrl driver user and itself to be
>>> built as module;
>>> - Use function callbacks for SCU related functions in pinctrl-imx.c
>>> in order to support the scenario of PINCTRL_IMX is built in
>>> while PINCTRL_IMX_SCU is built as module;
>>> - All drivers using SCU pinctrl driver need to initialize the
>>> SCU related function callback;
>>> - Change PINCTR_IMX_SCU to tristate;
>>> - Add module author, description and license.
>>>
>>> With above changes, i.MX SCU pinctrl driver can be built as module.
>>
>> There are a lot of changes here. I think it would be better to try to split them
>>
>> per functionality. One functional change per patch.
> Actually, I ever tried to split them, but the function will be broken. All the changes
> are just to support the module build. If split them, the bisect will have pinctrl
> build or function broken.
Hi Anson,
I see your point and I know that this is a very hard task to get it
right from
the first patches.
But let me suggest at least that:
- changes in drivers/pinctrl/freescale/pinctrl-imx.c (include file and
MODULE_ macros should go to a separate patch).
thanks
Daniel.
^ permalink raw reply
* RE: [PATCH 1/2] pinctrl: imx: Support building SCU pinctrl driver as module
From: Anson Huang @ 2020-07-16 15:21 UTC (permalink / raw)
To: Daniel Baluta, Aisheng Dong, festevam@gmail.com,
shawnguo@kernel.org, stefan@agner.ch, kernel@pengutronix.de,
linus.walleij@linaro.org, s.hauer@pengutronix.de,
linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
Cc: dl-linux-imx
In-Reply-To: <c00f8fe3-d12a-0f91-c301-c028e5aa3f25@nxp.com>
Hi, Daniel
> Subject: Re: [PATCH 1/2] pinctrl: imx: Support building SCU pinctrl driver as
> module
>
> Hi Anson,
>
> Few comments inline:
>
> On 7/16/20 6:06 PM, Anson Huang wrote:
> > To support building i.MX SCU pinctrl driver as module, below things need to
> be changed:
> >
> > - Export SCU related functions and use "IS_ENABLED" instead of
> > "ifdef" to support SCU pinctrl driver user and itself to be
> > built as module;
> > - Use function callbacks for SCU related functions in pinctrl-imx.c
> > in order to support the scenario of PINCTRL_IMX is built in
> > while PINCTRL_IMX_SCU is built as module;
> > - All drivers using SCU pinctrl driver need to initialize the
> > SCU related function callback;
> > - Change PINCTR_IMX_SCU to tristate;
> > - Add module author, description and license.
> >
> > With above changes, i.MX SCU pinctrl driver can be built as module.
>
>
> There are a lot of changes here. I think it would be better to try to split them
>
> per functionality. One functional change per patch.
Actually, I ever tried to split them, but the function will be broken. All the changes
are just to support the module build. If split them, the bisect will have pinctrl
build or function broken.
Thanks,
Anson
^ permalink raw reply
* Re: [PATCH 1/2] pinctrl: imx: Support building SCU pinctrl driver as module
From: Daniel Baluta @ 2020-07-16 15:14 UTC (permalink / raw)
To: Anson Huang, aisheng.dong, festevam, shawnguo, stefan, kernel,
linus.walleij, s.hauer, linux-gpio, linux-kernel,
linux-arm-kernel
Cc: Linux-imx
In-Reply-To: <1594912013-20859-1-git-send-email-Anson.Huang@nxp.com>
Hi Anson,
Few comments inline:
On 7/16/20 6:06 PM, Anson Huang wrote:
> To support building i.MX SCU pinctrl driver as module, below things need to be changed:
>
> - Export SCU related functions and use "IS_ENABLED" instead of
> "ifdef" to support SCU pinctrl driver user and itself to be
> built as module;
> - Use function callbacks for SCU related functions in pinctrl-imx.c
> in order to support the scenario of PINCTRL_IMX is built in
> while PINCTRL_IMX_SCU is built as module;
> - All drivers using SCU pinctrl driver need to initialize the
> SCU related function callback;
> - Change PINCTR_IMX_SCU to tristate;
> - Add module author, description and license.
>
> With above changes, i.MX SCU pinctrl driver can be built as module.
There are a lot of changes here. I think it would be better to try to
split them
per functionality. One functional change per patch.
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> drivers/pinctrl/freescale/Kconfig | 2 +-
> drivers/pinctrl/freescale/pinctrl-imx.c | 8 ++--
> drivers/pinctrl/freescale/pinctrl-imx.h | 57 ++++++++++++-----------------
> drivers/pinctrl/freescale/pinctrl-imx8dxl.c | 3 ++
> drivers/pinctrl/freescale/pinctrl-imx8qm.c | 3 ++
> drivers/pinctrl/freescale/pinctrl-imx8qxp.c | 3 ++
> drivers/pinctrl/freescale/pinctrl-scu.c | 5 +++
> 7 files changed, 42 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/pinctrl/freescale/Kconfig b/drivers/pinctrl/freescale/Kconfig
> index 08fcf5c..570355c 100644
> --- a/drivers/pinctrl/freescale/Kconfig
> +++ b/drivers/pinctrl/freescale/Kconfig
> @@ -7,7 +7,7 @@ config PINCTRL_IMX
> select REGMAP
>
> config PINCTRL_IMX_SCU
> - bool
> + tristate "IMX SCU pinctrl driver"
> depends on IMX_SCU
> select PINCTRL_IMX
>
> diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
> index 507e4af..b80c450 100644
> --- a/drivers/pinctrl/freescale/pinctrl-imx.c
> +++ b/drivers/pinctrl/freescale/pinctrl-imx.c
> @@ -373,7 +373,7 @@ static int imx_pinconf_get(struct pinctrl_dev *pctldev,
> const struct imx_pinctrl_soc_info *info = ipctl->info;
>
> if (info->flags & IMX_USE_SCU)
> - return imx_pinconf_get_scu(pctldev, pin_id, config);
> + return info->imx_pinconf_get(pctldev, pin_id, config);
> else
> return imx_pinconf_get_mmio(pctldev, pin_id, config);
> }
> @@ -423,7 +423,7 @@ static int imx_pinconf_set(struct pinctrl_dev *pctldev,
> const struct imx_pinctrl_soc_info *info = ipctl->info;
>
> if (info->flags & IMX_USE_SCU)
> - return imx_pinconf_set_scu(pctldev, pin_id,
> + return info->imx_pinconf_set(pctldev, pin_id,
> configs, num_configs);
> else
> return imx_pinconf_set_mmio(pctldev, pin_id,
> @@ -440,7 +440,7 @@ static void imx_pinconf_dbg_show(struct pinctrl_dev *pctldev,
> int ret;
>
> if (info->flags & IMX_USE_SCU) {
> - ret = imx_pinconf_get_scu(pctldev, pin_id, &config);
> + ret = info->imx_pinconf_get(pctldev, pin_id, &config);
> if (ret) {
> dev_err(ipctl->dev, "failed to get %s pinconf\n",
> pin_get_name(pctldev, pin_id));
> @@ -629,7 +629,7 @@ static int imx_pinctrl_parse_groups(struct device_node *np,
> for (i = 0; i < grp->num_pins; i++) {
> pin = &((struct imx_pin *)(grp->data))[i];
> if (info->flags & IMX_USE_SCU)
> - imx_pinctrl_parse_pin_scu(ipctl, &grp->pins[i],
> + info->imx_pinctrl_parse_pin(ipctl, &grp->pins[i],
> pin, &list);
> else
> imx_pinctrl_parse_pin_mmio(ipctl, &grp->pins[i],
> diff --git a/drivers/pinctrl/freescale/pinctrl-imx.h b/drivers/pinctrl/freescale/pinctrl-imx.h
> index 333d32b..bdb86c2 100644
> --- a/drivers/pinctrl/freescale/pinctrl-imx.h
> +++ b/drivers/pinctrl/freescale/pinctrl-imx.h
> @@ -75,6 +75,21 @@ struct imx_cfg_params_decode {
> bool invert;
> };
>
> +/**
> + * @dev: a pointer back to containing device
> + * @base: the offset to the controller in virtual memory
> + */
> +struct imx_pinctrl {
> + struct device *dev;
> + struct pinctrl_dev *pctl;
> + void __iomem *base;
> + void __iomem *input_sel_base;
> + const struct imx_pinctrl_soc_info *info;
> + struct imx_pin_reg *pin_regs;
> + unsigned int group_index;
> + struct mutex mutex;
> +};
> +
> struct imx_pinctrl_soc_info {
> const struct pinctrl_pin_desc *pins;
> unsigned int npins;
> @@ -98,21 +113,13 @@ struct imx_pinctrl_soc_info {
> struct pinctrl_gpio_range *range,
> unsigned offset,
> bool input);
> -};
> -
> -/**
> - * @dev: a pointer back to containing device
> - * @base: the offset to the controller in virtual memory
> - */
> -struct imx_pinctrl {
> - struct device *dev;
> - struct pinctrl_dev *pctl;
> - void __iomem *base;
> - void __iomem *input_sel_base;
> - const struct imx_pinctrl_soc_info *info;
> - struct imx_pin_reg *pin_regs;
> - unsigned int group_index;
> - struct mutex mutex;
> + int (*imx_pinconf_get)(struct pinctrl_dev *pctldev, unsigned int pin_id,
> + unsigned long *config);
> + int (*imx_pinconf_set)(struct pinctrl_dev *pctldev, unsigned int pin_id,
> + unsigned long *configs, unsigned int num_configs);
> + void (*imx_pinctrl_parse_pin)(struct imx_pinctrl *ipctl,
> + unsigned int *pin_id, struct imx_pin *pin,
> + const __be32 **list_p);
> };
>
> #define IMX_CFG_PARAMS_DECODE(p, m, o) \
> @@ -137,7 +144,7 @@ struct imx_pinctrl {
> int imx_pinctrl_probe(struct platform_device *pdev,
> const struct imx_pinctrl_soc_info *info);
>
> -#ifdef CONFIG_PINCTRL_IMX_SCU
> +#if IS_ENABLED(CONFIG_PINCTRL_IMX_SCU)
> #define BM_PAD_CTL_GP_ENABLE BIT(30)
> #define BM_PAD_CTL_IFMUX_ENABLE BIT(31)
> #define BP_PAD_CTL_IFMUX 27
> @@ -150,23 +157,5 @@ int imx_pinconf_set_scu(struct pinctrl_dev *pctldev, unsigned pin_id,
> void imx_pinctrl_parse_pin_scu(struct imx_pinctrl *ipctl,
> unsigned int *pin_id, struct imx_pin *pin,
> const __be32 **list_p);
> -#else
> -static inline int imx_pinconf_get_scu(struct pinctrl_dev *pctldev,
> - unsigned pin_id, unsigned long *config)
> -{
> - return -EINVAL;
> -}
> -static inline int imx_pinconf_set_scu(struct pinctrl_dev *pctldev,
> - unsigned pin_id, unsigned long *configs,
> - unsigned num_configs)
> -{
> - return -EINVAL;
> -}
> -static inline void imx_pinctrl_parse_pin_scu(struct imx_pinctrl *ipctl,
> - unsigned int *pin_id,
> - struct imx_pin *pin,
> - const __be32 **list_p)
> -{
> -}
> #endif
> #endif /* __DRIVERS_PINCTRL_IMX_H */
> diff --git a/drivers/pinctrl/freescale/pinctrl-imx8dxl.c b/drivers/pinctrl/freescale/pinctrl-imx8dxl.c
> index 12b97da..d3020c0 100644
> --- a/drivers/pinctrl/freescale/pinctrl-imx8dxl.c
> +++ b/drivers/pinctrl/freescale/pinctrl-imx8dxl.c
> @@ -159,6 +159,9 @@ static struct imx_pinctrl_soc_info imx8dxl_pinctrl_info = {
> .pins = imx8dxl_pinctrl_pads,
> .npins = ARRAY_SIZE(imx8dxl_pinctrl_pads),
> .flags = IMX_USE_SCU,
> + .imx_pinconf_get = imx_pinconf_get_scu,
> + .imx_pinconf_set = imx_pinconf_set_scu,
> + .imx_pinctrl_parse_pin = imx_pinctrl_parse_pin_scu,
> };
>
> static const struct of_device_id imx8dxl_pinctrl_of_match[] = {
> diff --git a/drivers/pinctrl/freescale/pinctrl-imx8qm.c b/drivers/pinctrl/freescale/pinctrl-imx8qm.c
> index 095acf4..8f46b940 100644
> --- a/drivers/pinctrl/freescale/pinctrl-imx8qm.c
> +++ b/drivers/pinctrl/freescale/pinctrl-imx8qm.c
> @@ -292,6 +292,9 @@ static const struct imx_pinctrl_soc_info imx8qm_pinctrl_info = {
> .pins = imx8qm_pinctrl_pads,
> .npins = ARRAY_SIZE(imx8qm_pinctrl_pads),
> .flags = IMX_USE_SCU,
> + .imx_pinconf_get = imx_pinconf_get_scu,
> + .imx_pinconf_set = imx_pinconf_set_scu,
> + .imx_pinctrl_parse_pin = imx_pinctrl_parse_pin_scu,
> };
>
> static const struct of_device_id imx8qm_pinctrl_of_match[] = {
> diff --git a/drivers/pinctrl/freescale/pinctrl-imx8qxp.c b/drivers/pinctrl/freescale/pinctrl-imx8qxp.c
> index 81ebd4c..6776ad6 100644
> --- a/drivers/pinctrl/freescale/pinctrl-imx8qxp.c
> +++ b/drivers/pinctrl/freescale/pinctrl-imx8qxp.c
> @@ -198,6 +198,9 @@ static struct imx_pinctrl_soc_info imx8qxp_pinctrl_info = {
> .pins = imx8qxp_pinctrl_pads,
> .npins = ARRAY_SIZE(imx8qxp_pinctrl_pads),
> .flags = IMX_USE_SCU,
> + .imx_pinconf_get = imx_pinconf_get_scu,
> + .imx_pinconf_set = imx_pinconf_set_scu,
> + .imx_pinctrl_parse_pin = imx_pinctrl_parse_pin_scu,
> };
>
> static const struct of_device_id imx8qxp_pinctrl_of_match[] = {
> diff --git a/drivers/pinctrl/freescale/pinctrl-scu.c b/drivers/pinctrl/freescale/pinctrl-scu.c
> index 9df45d3..59b5f8a 100644
> --- a/drivers/pinctrl/freescale/pinctrl-scu.c
> +++ b/drivers/pinctrl/freescale/pinctrl-scu.c
> @@ -7,6 +7,7 @@
>
> #include <linux/err.h>
> #include <linux/firmware/imx/sci.h>
> +#include <linux/module.h>
> #include <linux/of_address.h>
> #include <linux/pinctrl/pinctrl.h>
> #include <linux/platform_device.h>
> @@ -123,3 +124,7 @@ void imx_pinctrl_parse_pin_scu(struct imx_pinctrl *ipctl,
> pin_scu->mux_mode, pin_scu->config);
> }
> EXPORT_SYMBOL_GPL(imx_pinctrl_parse_pin_scu);
> +
> +MODULE_AUTHOR("Dong Aisheng <aisheng.dong@nxp.com>");
> +MODULE_DESCRIPTION("NXP i.MX SCU common pinctrl driver");
> +MODULE_LICENSE("GPL v2");
^ permalink raw reply
* Re: [RFC v2 GPIO lines [was: GPIO User I/O]
From: Rodolfo Giometti @ 2020-07-16 15:15 UTC (permalink / raw)
To: Linus Walleij
Cc: Geert Uytterhoeven, Geert Uytterhoeven, open list:GPIO SUBSYSTEM,
Bartosz Golaszewski, Rob Herring,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
In-Reply-To: <CACRpkdZBUw5UPyZB-aeVwh8-GiCifbwABZ9mOsyK90t3cdMQ+w@mail.gmail.com>
On 16/07/2020 15:38, Linus Walleij wrote:
> On Tue, Jul 14, 2020 at 4:01 PM Rodolfo Giometti <giometti@enneenne.com> wrote:
>
>> I see... however attached is a new version of my proposal patch
>
> I looked a bit at this!
>
> IIUC the idea is a "new" sysfs interface that does not require the exporting
> etc used by the current "old" sysfs interface. Instead of poking around in
> sysfs to export lines we do that from the device tree.
Yes.
> It also does not use any global GPIO numbers which would be my other
> main concern.
Exactly, the idea is to have "names" that describe the IO lines to the userspace
and a way to fix their usage in each board in the device tree. If a board has a
relay line is a non-sense allow users in the userland to use it as an input line.
> I must admit that it has some elegance to it. Especially when it comes
> to scripting.
:)
> The problem I see is that lines are left in whatever state they were in
> if a script crashes, so there is no "return to the initial value" that was
> there when the GPIOs were picked from the device tree. This makes
> this a bit fragile.
I see but this interface is not designed for such complex usage nor to compete
with the current character interface! It is designed to allow boards
manufactures to "describe" some I/O lines that are not used by any driver in the
device tree, and that users may desire to manage in a very simple manner. Let's
thing about relay lines, or just a locked/unlocked input line which can be
easily polled.
> Also users regularly need to listen to events. This interface can and
> should never support that, for this one must use the character device,
> which will of course not work in parallel with using this sysfs ABI.
> And the day someone wants that we simply have to say no. There
> is no way to hold states for event handling in a stateless ABI.
>
> Well of course they can poll for a line to change, but that is not
> proper event handling that reacts to an interrupt.
Again, the basic idea is not to compete with the current character interface nor
to manage interrupts lines, but just to have a really simple way to describe
those I/O lines which at the moment has no name nor references within the device
tree.
> So while this is much more elegant the old sysfs ABI, and certainly
> better for scripting, it still suffers from some conflicts with
> the character device, and there is a risk to make users dissatisfied
> when they want to e.g. script event handlers.
>
> What are your thoughts on this?
Regarding the event handlers we can add irq supports... however I prefer to keep
this interface really simple just to not be confused with the character interface.
Ciao,
Rodolfo
--
GNU/Linux Solutions e-mail: giometti@enneenne.com
Linux Device Driver giometti@linux.it
Embedded Systems phone: +39 349 2432127
UNIX programming skype: rodolfo.giometti
^ permalink raw reply
* [PATCH 1/2] pinctrl: imx: Support building SCU pinctrl driver as module
From: Anson Huang @ 2020-07-16 15:06 UTC (permalink / raw)
To: aisheng.dong, festevam, shawnguo, stefan, kernel, linus.walleij,
s.hauer, linux-gpio, linux-kernel, linux-arm-kernel
Cc: Linux-imx
To support building i.MX SCU pinctrl driver as module, below things need to be changed:
- Export SCU related functions and use "IS_ENABLED" instead of
"ifdef" to support SCU pinctrl driver user and itself to be
built as module;
- Use function callbacks for SCU related functions in pinctrl-imx.c
in order to support the scenario of PINCTRL_IMX is built in
while PINCTRL_IMX_SCU is built as module;
- All drivers using SCU pinctrl driver need to initialize the
SCU related function callback;
- Change PINCTR_IMX_SCU to tristate;
- Add module author, description and license.
With above changes, i.MX SCU pinctrl driver can be built as module.
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
drivers/pinctrl/freescale/Kconfig | 2 +-
drivers/pinctrl/freescale/pinctrl-imx.c | 8 ++--
drivers/pinctrl/freescale/pinctrl-imx.h | 57 ++++++++++++-----------------
drivers/pinctrl/freescale/pinctrl-imx8dxl.c | 3 ++
drivers/pinctrl/freescale/pinctrl-imx8qm.c | 3 ++
drivers/pinctrl/freescale/pinctrl-imx8qxp.c | 3 ++
drivers/pinctrl/freescale/pinctrl-scu.c | 5 +++
7 files changed, 42 insertions(+), 39 deletions(-)
diff --git a/drivers/pinctrl/freescale/Kconfig b/drivers/pinctrl/freescale/Kconfig
index 08fcf5c..570355c 100644
--- a/drivers/pinctrl/freescale/Kconfig
+++ b/drivers/pinctrl/freescale/Kconfig
@@ -7,7 +7,7 @@ config PINCTRL_IMX
select REGMAP
config PINCTRL_IMX_SCU
- bool
+ tristate "IMX SCU pinctrl driver"
depends on IMX_SCU
select PINCTRL_IMX
diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
index 507e4af..b80c450 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx.c
@@ -373,7 +373,7 @@ static int imx_pinconf_get(struct pinctrl_dev *pctldev,
const struct imx_pinctrl_soc_info *info = ipctl->info;
if (info->flags & IMX_USE_SCU)
- return imx_pinconf_get_scu(pctldev, pin_id, config);
+ return info->imx_pinconf_get(pctldev, pin_id, config);
else
return imx_pinconf_get_mmio(pctldev, pin_id, config);
}
@@ -423,7 +423,7 @@ static int imx_pinconf_set(struct pinctrl_dev *pctldev,
const struct imx_pinctrl_soc_info *info = ipctl->info;
if (info->flags & IMX_USE_SCU)
- return imx_pinconf_set_scu(pctldev, pin_id,
+ return info->imx_pinconf_set(pctldev, pin_id,
configs, num_configs);
else
return imx_pinconf_set_mmio(pctldev, pin_id,
@@ -440,7 +440,7 @@ static void imx_pinconf_dbg_show(struct pinctrl_dev *pctldev,
int ret;
if (info->flags & IMX_USE_SCU) {
- ret = imx_pinconf_get_scu(pctldev, pin_id, &config);
+ ret = info->imx_pinconf_get(pctldev, pin_id, &config);
if (ret) {
dev_err(ipctl->dev, "failed to get %s pinconf\n",
pin_get_name(pctldev, pin_id));
@@ -629,7 +629,7 @@ static int imx_pinctrl_parse_groups(struct device_node *np,
for (i = 0; i < grp->num_pins; i++) {
pin = &((struct imx_pin *)(grp->data))[i];
if (info->flags & IMX_USE_SCU)
- imx_pinctrl_parse_pin_scu(ipctl, &grp->pins[i],
+ info->imx_pinctrl_parse_pin(ipctl, &grp->pins[i],
pin, &list);
else
imx_pinctrl_parse_pin_mmio(ipctl, &grp->pins[i],
diff --git a/drivers/pinctrl/freescale/pinctrl-imx.h b/drivers/pinctrl/freescale/pinctrl-imx.h
index 333d32b..bdb86c2 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx.h
+++ b/drivers/pinctrl/freescale/pinctrl-imx.h
@@ -75,6 +75,21 @@ struct imx_cfg_params_decode {
bool invert;
};
+/**
+ * @dev: a pointer back to containing device
+ * @base: the offset to the controller in virtual memory
+ */
+struct imx_pinctrl {
+ struct device *dev;
+ struct pinctrl_dev *pctl;
+ void __iomem *base;
+ void __iomem *input_sel_base;
+ const struct imx_pinctrl_soc_info *info;
+ struct imx_pin_reg *pin_regs;
+ unsigned int group_index;
+ struct mutex mutex;
+};
+
struct imx_pinctrl_soc_info {
const struct pinctrl_pin_desc *pins;
unsigned int npins;
@@ -98,21 +113,13 @@ struct imx_pinctrl_soc_info {
struct pinctrl_gpio_range *range,
unsigned offset,
bool input);
-};
-
-/**
- * @dev: a pointer back to containing device
- * @base: the offset to the controller in virtual memory
- */
-struct imx_pinctrl {
- struct device *dev;
- struct pinctrl_dev *pctl;
- void __iomem *base;
- void __iomem *input_sel_base;
- const struct imx_pinctrl_soc_info *info;
- struct imx_pin_reg *pin_regs;
- unsigned int group_index;
- struct mutex mutex;
+ int (*imx_pinconf_get)(struct pinctrl_dev *pctldev, unsigned int pin_id,
+ unsigned long *config);
+ int (*imx_pinconf_set)(struct pinctrl_dev *pctldev, unsigned int pin_id,
+ unsigned long *configs, unsigned int num_configs);
+ void (*imx_pinctrl_parse_pin)(struct imx_pinctrl *ipctl,
+ unsigned int *pin_id, struct imx_pin *pin,
+ const __be32 **list_p);
};
#define IMX_CFG_PARAMS_DECODE(p, m, o) \
@@ -137,7 +144,7 @@ struct imx_pinctrl {
int imx_pinctrl_probe(struct platform_device *pdev,
const struct imx_pinctrl_soc_info *info);
-#ifdef CONFIG_PINCTRL_IMX_SCU
+#if IS_ENABLED(CONFIG_PINCTRL_IMX_SCU)
#define BM_PAD_CTL_GP_ENABLE BIT(30)
#define BM_PAD_CTL_IFMUX_ENABLE BIT(31)
#define BP_PAD_CTL_IFMUX 27
@@ -150,23 +157,5 @@ int imx_pinconf_set_scu(struct pinctrl_dev *pctldev, unsigned pin_id,
void imx_pinctrl_parse_pin_scu(struct imx_pinctrl *ipctl,
unsigned int *pin_id, struct imx_pin *pin,
const __be32 **list_p);
-#else
-static inline int imx_pinconf_get_scu(struct pinctrl_dev *pctldev,
- unsigned pin_id, unsigned long *config)
-{
- return -EINVAL;
-}
-static inline int imx_pinconf_set_scu(struct pinctrl_dev *pctldev,
- unsigned pin_id, unsigned long *configs,
- unsigned num_configs)
-{
- return -EINVAL;
-}
-static inline void imx_pinctrl_parse_pin_scu(struct imx_pinctrl *ipctl,
- unsigned int *pin_id,
- struct imx_pin *pin,
- const __be32 **list_p)
-{
-}
#endif
#endif /* __DRIVERS_PINCTRL_IMX_H */
diff --git a/drivers/pinctrl/freescale/pinctrl-imx8dxl.c b/drivers/pinctrl/freescale/pinctrl-imx8dxl.c
index 12b97da..d3020c0 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx8dxl.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx8dxl.c
@@ -159,6 +159,9 @@ static struct imx_pinctrl_soc_info imx8dxl_pinctrl_info = {
.pins = imx8dxl_pinctrl_pads,
.npins = ARRAY_SIZE(imx8dxl_pinctrl_pads),
.flags = IMX_USE_SCU,
+ .imx_pinconf_get = imx_pinconf_get_scu,
+ .imx_pinconf_set = imx_pinconf_set_scu,
+ .imx_pinctrl_parse_pin = imx_pinctrl_parse_pin_scu,
};
static const struct of_device_id imx8dxl_pinctrl_of_match[] = {
diff --git a/drivers/pinctrl/freescale/pinctrl-imx8qm.c b/drivers/pinctrl/freescale/pinctrl-imx8qm.c
index 095acf4..8f46b940 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx8qm.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx8qm.c
@@ -292,6 +292,9 @@ static const struct imx_pinctrl_soc_info imx8qm_pinctrl_info = {
.pins = imx8qm_pinctrl_pads,
.npins = ARRAY_SIZE(imx8qm_pinctrl_pads),
.flags = IMX_USE_SCU,
+ .imx_pinconf_get = imx_pinconf_get_scu,
+ .imx_pinconf_set = imx_pinconf_set_scu,
+ .imx_pinctrl_parse_pin = imx_pinctrl_parse_pin_scu,
};
static const struct of_device_id imx8qm_pinctrl_of_match[] = {
diff --git a/drivers/pinctrl/freescale/pinctrl-imx8qxp.c b/drivers/pinctrl/freescale/pinctrl-imx8qxp.c
index 81ebd4c..6776ad6 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx8qxp.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx8qxp.c
@@ -198,6 +198,9 @@ static struct imx_pinctrl_soc_info imx8qxp_pinctrl_info = {
.pins = imx8qxp_pinctrl_pads,
.npins = ARRAY_SIZE(imx8qxp_pinctrl_pads),
.flags = IMX_USE_SCU,
+ .imx_pinconf_get = imx_pinconf_get_scu,
+ .imx_pinconf_set = imx_pinconf_set_scu,
+ .imx_pinctrl_parse_pin = imx_pinctrl_parse_pin_scu,
};
static const struct of_device_id imx8qxp_pinctrl_of_match[] = {
diff --git a/drivers/pinctrl/freescale/pinctrl-scu.c b/drivers/pinctrl/freescale/pinctrl-scu.c
index 9df45d3..59b5f8a 100644
--- a/drivers/pinctrl/freescale/pinctrl-scu.c
+++ b/drivers/pinctrl/freescale/pinctrl-scu.c
@@ -7,6 +7,7 @@
#include <linux/err.h>
#include <linux/firmware/imx/sci.h>
+#include <linux/module.h>
#include <linux/of_address.h>
#include <linux/pinctrl/pinctrl.h>
#include <linux/platform_device.h>
@@ -123,3 +124,7 @@ void imx_pinctrl_parse_pin_scu(struct imx_pinctrl *ipctl,
pin_scu->mux_mode, pin_scu->config);
}
EXPORT_SYMBOL_GPL(imx_pinctrl_parse_pin_scu);
+
+MODULE_AUTHOR("Dong Aisheng <aisheng.dong@nxp.com>");
+MODULE_DESCRIPTION("NXP i.MX SCU common pinctrl driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4
^ permalink raw reply related
* [PATCH 2/2] pinctrl: imx: Support building i.MX pinctrl driver as module
From: Anson Huang @ 2020-07-16 15:06 UTC (permalink / raw)
To: aisheng.dong, festevam, shawnguo, stefan, kernel, linus.walleij,
s.hauer, linux-gpio, linux-kernel, linux-arm-kernel
Cc: Linux-imx
In-Reply-To: <1594912013-20859-1-git-send-email-Anson.Huang@nxp.com>
Change PINCTRL_IMX to tristate to support loadable module build.
And i.MX common pinctrl driver should depend on CONFIG_OF to make sure
no build error when i.MX common pinctrl driver is enabled for different
architectures without CONFIG_OF.
Also add module author, description and license.
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
drivers/pinctrl/freescale/Kconfig | 3 ++-
drivers/pinctrl/freescale/pinctrl-imx.c | 5 +++++
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/pinctrl/freescale/Kconfig b/drivers/pinctrl/freescale/Kconfig
index 570355c..922ae4b 100644
--- a/drivers/pinctrl/freescale/Kconfig
+++ b/drivers/pinctrl/freescale/Kconfig
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-2.0-only
config PINCTRL_IMX
- bool
+ tristate "IMX pinctrl driver"
+ depends on OF
select GENERIC_PINCTRL_GROUPS
select GENERIC_PINMUX_FUNCTIONS
select GENERIC_PINCONF
diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
index b80c450..3eaafb6 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx.c
@@ -11,6 +11,7 @@
#include <linux/init.h>
#include <linux/io.h>
#include <linux/mfd/syscon.h>
+#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_device.h>
#include <linux/of_address.h>
@@ -898,3 +899,7 @@ const struct dev_pm_ops imx_pinctrl_pm_ops = {
imx_pinctrl_resume)
};
EXPORT_SYMBOL_GPL(imx_pinctrl_pm_ops);
+
+MODULE_AUTHOR("Linus Walleij <linus.walleij@linaro.org>");
+MODULE_DESCRIPTION("NXP i.MX common pinctrl driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v3 10/16] mfd: axp20x: Allow the AXP803 to be probed by I2C
From: Lee Jones @ 2020-07-16 15:10 UTC (permalink / raw)
To: Frank Lee
Cc: robh+dt, mripard, wens, mturquette, sboyd, gregory.clement, tglx,
jason, maz, srinivas.kandagatla, linus.walleij, anarsoul,
tiny.windzz, rui.zhang, daniel.lezcano, amit.kucheria, p.zabel,
clabbe, icenowy, megous, stefan, bage, devicetree,
linux-arm-kernel, linux-kernel, linux-clk, linux-i2c, linux-gpio,
linux-pm, huangshuosheng, liyong
In-Reply-To: <20200708071942.22595-11-frank@allwinnertech.com>
On Wed, 08 Jul 2020, Frank Lee wrote:
> The AXP803 can be used both using the RSB proprietary bus, or a more
> traditional I2C bus.
>
> Let's add that possibility.
>
> Signed-off-by: Frank Lee <frank@allwinnertech.com>
> ---
> drivers/mfd/axp20x-i2c.c | 2 ++
> 1 file changed, 2 insertions(+)
Applied, thanks.
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [PATCH] gpio: adp5588: Use irqchip template
From: Linus Walleij @ 2020-07-16 15:05 UTC (permalink / raw)
To: linux-gpio
Cc: Bartosz Golaszewski, Linus Walleij, Nikolaus Voss,
Michael Hennerich
This makes the driver use the irqchip template to assign
properties to the gpio_irq_chip instead of using the
explicit calls to gpiochip_irqchip_add_nested() and
gpiochip_set_nested_irqchip(). The irqchip is instead
added while adding the gpiochip.
Cc: Nikolaus Voss <nv@vosn.de>
Cc: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/gpio/gpio-adp5588.c | 39 +++++++++++++++++++++++--------------
1 file changed, 24 insertions(+), 15 deletions(-)
diff --git a/drivers/gpio/gpio-adp5588.c b/drivers/gpio/gpio-adp5588.c
index 49f423d7beba..f1e4ac90e7d3 100644
--- a/drivers/gpio/gpio-adp5588.c
+++ b/drivers/gpio/gpio-adp5588.c
@@ -272,13 +272,24 @@ static irqreturn_t adp5588_irq_handler(int irq, void *devid)
return IRQ_HANDLED;
}
+
+static int adp5588_irq_init_hw(struct gpio_chip *gc)
+{
+ struct adp5588_gpio *dev = gpiochip_get_data(gc);
+ /* Enable IRQs after registering chip */
+ adp5588_gpio_write(dev->client, CFG,
+ ADP5588_AUTO_INC | ADP5588_INT_CFG | ADP5588_KE_IEN);
+
+ return 0;
+}
+
static int adp5588_irq_setup(struct adp5588_gpio *dev)
{
struct i2c_client *client = dev->client;
int ret;
struct adp5588_gpio_platform_data *pdata =
dev_get_platdata(&client->dev);
- int irq_base = pdata ? pdata->irq_base : 0;
+ struct gpio_irq_chip *girq;
adp5588_gpio_write(client, CFG, ADP5588_AUTO_INC);
adp5588_gpio_write(client, INT_STAT, -1); /* status is W1C */
@@ -294,21 +305,19 @@ static int adp5588_irq_setup(struct adp5588_gpio *dev)
client->irq);
return ret;
}
- ret = gpiochip_irqchip_add_nested(&dev->gpio_chip,
- &adp5588_irq_chip, irq_base,
- handle_simple_irq,
- IRQ_TYPE_NONE);
- if (ret) {
- dev_err(&client->dev,
- "could not connect irqchip to gpiochip\n");
- return ret;
- }
- gpiochip_set_nested_irqchip(&dev->gpio_chip,
- &adp5588_irq_chip,
- client->irq);
- adp5588_gpio_write(client, CFG,
- ADP5588_AUTO_INC | ADP5588_INT_CFG | ADP5588_KE_IEN);
+ /* This will be registered in the call to devm_gpiochip_add_data() */
+ girq = &dev->gpio_chip.irq;
+ girq->chip = &adp5588_irq_chip;
+ /* This will let us handle the parent IRQ in the driver */
+ girq->parent_handler = NULL;
+ girq->num_parents = 0;
+ girq->parents = NULL;
+ girq->first = pdata ? pdata->irq_base : 0;
+ girq->default_type = IRQ_TYPE_NONE;
+ girq->handler = handle_simple_irq;
+ girq->init_hw = adp5588_irq_init_hw;
+ girq->threaded = true;
return 0;
}
--
2.26.2
^ permalink raw reply related
* Re: [PATCH 1/3] dt-bindings: pinctrl: Add bindings for Actions S500 SoC
From: Rob Herring @ 2020-07-16 14:50 UTC (permalink / raw)
To: Cristian Ciocaltea
Cc: Andreas Färber, Manivannan Sadhasivam, Linus Walleij,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
open list:GPIO SUBSYSTEM, devicetree,
linux-kernel@vger.kernel.org, linux-actions
In-Reply-To: <20200716104316.GA309338@BV030612LT>
On Thu, Jul 16, 2020 at 4:43 AM Cristian Ciocaltea
<cristian.ciocaltea@gmail.com> wrote:
>
> On Wed, Jul 15, 2020 at 02:03:09PM -0600, Rob Herring wrote:
> > On Thu, Jun 25, 2020 at 11:16:18PM +0300, Cristian Ciocaltea wrote:
> > > Add pinctrl and gpio bindings for Actions Semi S500 SoC.
> > >
> > > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com>
> > > ---
> > > .../pinctrl/actions,s500-pinctrl.yaml | 228 ++++++++++++++++++
> > > 1 file changed, 228 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/pinctrl/actions,s500-pinctrl.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/pinctrl/actions,s500-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/actions,s500-pinctrl.yaml
> > > new file mode 100644
> > > index 000000000000..856947c70844
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/pinctrl/actions,s500-pinctrl.yaml
> > > @@ -0,0 +1,228 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/pinctrl/actions,s500-pinctrl.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Actions Semi S500 SoC pinmux & GPIO controller
> > > +
> > > +maintainers:
> > > + - Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > +
> > > +description: |
> > > + Pinmux & GPIO controller manages pin multiplexing & configuration including
> > > + GPIO function selection & GPIO attributes configuration. Please refer to
> > > + pinctrl-bindings.txt in this directory for common binding part and usage.
> > > +
> > > +properties:
> > > + compatible:
> > > + const: actions,s500-pinctrl
> > > +
> > > + reg:
> > > + minItems: 1
> > > + maxItems: 4
> >
> > Need to enumerate what each register range is.
>
> Hi Rob,
>
> Thanks for the review!
>
> Would the update below suffice?
>
> reg:
> description: |
> Specifies the memory region(s) associated with the pin-controller.
> To improve granularity, up to four register ranges can be provided:
What does 'improve granularity' mean:
> * GPIO Output + GPIO Input + GPIO Data
> * Multiplexing Control
> * PAD Pull Control + PAD Schmitt Trigger enable + PAD Control
> * PAD Drive Capacity Select
The h/w sometimes has these and sometimes doesn't?
If they do stay, then you want:
items:
- description: GPIO Output + GPIO Input + GPIO Data
- description: ...
>
> > > +
> > > + clocks:
> > > + maxItems: 1
> > > +
> > > + gpio-controller: true
> > > +
> > > + gpio-ranges:
> > > + maxItems: 1
> > > +
> > > + '#gpio-cells':
> > > + description:
> > > + Specifies the pin number and flags, as defined in
> > > + include/dt-bindings/gpio/gpio.h
> > > + const: 2
> > > +
> > > + interrupt-controller: true
> > > +
> > > + '#interrupt-cells':
> > > + description:
> > > + Specifies the pin number and flags, as defined in
> > > + include/dt-bindings/interrupt-controller/irq.h
> > > + const: 2
> > > +
> > > + interrupts:
> > > + description:
> > > + One interrupt per each of the 5 GPIO ports supported by the controller,
> > > + sorted by port number ascending order.
> > > + minItems: 5
> > > + maxItems: 5
> > > +
> > > +patternProperties:
> > > + '^.*$':
> > > + if:
> > > + type: object
> >
> > For a new binding, can you do '-pins$' for the node names so we don't
> > need this if/then hack.
>
> Right, the idea was to be consistent with the existing bindings for
> S700 and S900, which allow free node names, although they are not yet
> converted to yaml format.
If we want consistency, those should have their node names updated.
>
> > > + then:
> > > + patternProperties:
> > > + 'pinmux$':
> >
> > Is this really a pattern? Can't tell from the example.
>
> pinmux and pinconf subnodes may appear multiple times, that's why I
> decided to match their names based on the suffix.
>
> The example is not complex enough, I will change it to the following:
>
> mmc0_default: mmc0_default {
> pinmux {
> groups = "sd0_d0_mfp", "sd0_d1_mfp", "sd0_d2_d3_mfp",
> "sd0_cmd_mfp", "sd0_clk_mfp";
> function = "sd0";
> };
>
> drv_pinconf {
drv-pinconf
Make the pattern '-?pinconf' to enforce that. (that '-' may need escaping?)
^ permalink raw reply
* RE: [PATCH 1/3] gpio: mxc: Support module build
From: Anson Huang @ 2020-07-16 14:37 UTC (permalink / raw)
To: Linus Walleij
Cc: Russell King, Shawn Guo, Sascha Hauer, Sascha Hauer,
Fabio Estevam, Catalin Marinas, Will Deacon, Bartosz Golaszewski,
oleksandr.suvorov@toradex.com, Adam Ford, Andreas Kemnade,
hverkuil-cisco@xs4all.nl, Bjorn Andersson, Leo Li, Vinod Koul,
Geert Uytterhoeven, Olof Johansson, Linux ARM,
linux-kernel@vger.kernel.org, open list:GPIO SUBSYSTEM,
dl-linux-imx
In-Reply-To: <CACRpkda2gdu8FsSM0MC6g8C1mebmVc5dFWJZwNvQUPXNi5bnkQ@mail.gmail.com>
Hi, Linus
> Subject: Re: [PATCH 1/3] gpio: mxc: Support module build
>
> On Wed, Jul 15, 2020 at 4:44 AM Anson Huang <anson.huang@nxp.com>
> wrote:
>
> > > Subject: RE: [PATCH 1/3] gpio: mxc: Support module build
> > >
> > > Hi, Linus
> > >
> > > > Subject: Re: [PATCH 1/3] gpio: mxc: Support module build
> > > >
> > > > On Wed, Jul 8, 2020 at 1:28 AM Anson Huang <Anson.Huang@nxp.com>
> > > > wrote:
> > > >
> > > > > subsys_initcall(gpio_mxc_init);
> > > > > +
> > > > > +MODULE_AUTHOR("Shawn Guo <shawn.guo@linaro.org>");
> > > > > +MODULE_DESCRIPTION("i.MX GPIO Driver");
> MODULE_LICENSE("GPL");
> > > >
> > > > You are making this modualrizable but keeping the
> > > > subsys_initcall(), which doesn't make very much sense. It is
> > > > obviously not necessary to do this probe at subsys_initcall() time, right?
> > > >
> > >
> > > If building it as module, the subsys_initcall() will be equal to
> > > module_init(), I keep it unchanged is because I try to make it
> > > identical when built-in, since most of the config will still have it built-in,
> except the Android GKI support.
> > > Does it make sense?
> > >
> > > > Take this opportunity to convert the driver to use
> > > > module_platform_driver() as well.
> > >
> > > If you think it has to be or it is better to use
> > > module_platform_driver(), I will do it in V2.
> >
> > I tried to replace the subsys_initcall() with
> > module_platform_driver(), but met issue about "
> > register_syscore_ops(&mxc_gpio_syscore_ops);" which is called in
> > gpio_mxc_init() function, this function should be called ONLY once,
> > moving it to .probe function is NOT working, so we may need to keep the
> gpio_mxc_init(), that is another reason that we may need to keep
> subsys_initcall()?
>
> This looks a bit dangerous to keep like this while allowing this code to be used
> from a module.
>
> What happens if you insmod and rmmod this a few times, really?
> How is this tested?
>
> This is not really modularized if that isn't working, just that modprobing once
> works isn't real modularization IMO, it seems more like a quick and dirty way
> to get Androids GKI somewhat working with the module while not properly
> making the module a module.
>
> You need input from the driver maintainers on how to handle this.
As far as I know, some general/critical modules are NOT supporting rmmod, like
clk, pinctrl, gpio etc., and I am NOT sure whether Android GKI need to support
rmmod for these system-wide-used module, I will ask them for more detail about
this.
The requirement I received is to support loadable module, but so far no hard requirement
to support module remove for gpio driver, so, is it OK to add it step by step, and this patch
series ONLY to support module build and one time modprobe?
Thanks,
Anson
^ permalink raw reply
* Re: [PATCH 20/25] pinctrl: pinctrl-rza1: Demote some kerneldoc headers and fix others
From: Geert Uytterhoeven @ 2020-07-16 14:12 UTC (permalink / raw)
To: Linus Walleij
Cc: Lee Jones, Linux Kernel Mailing List, open list:GPIO SUBSYSTEM,
Geert Uytterhoeven, Jacopo Mondi, Linux-Renesas
In-Reply-To: <CACRpkdYep_r1KsTnU2gVr-DeOf50hRiyTRq=jgeas=fD-qPHVg@mail.gmail.com>
Hi Linus,
On Thu, Jul 16, 2020 at 3:57 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Jul 15, 2020 at 9:30 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > i.e. will queue in sh-pfc-for-v5.9.
>
> OK since Geert is queueing this I'll drop this patch from my tree.
Oops, sorry, I didn't know you were planning to apply this one.
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
* Re: [PATCH QEMU v2 4/5] ARM: PL061: Add gpiodev support
From: Geert Uytterhoeven @ 2020-07-16 14:10 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Paolo Bonzini, Alexander Graf, Linus Walleij,
Bartosz Golaszewski, Magnus Damm, Linux-Renesas,
open list:GPIO SUBSYSTEM, qemu-arm, QEMU Developers
In-Reply-To: <e279f622-3af6-5073-dac0-4c452a88c32b@amsat.org>
Hi Philippe,
On Thu, Apr 23, 2020 at 12:08 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> On 4/23/20 11:33 AM, Philippe Mathieu-Daudé wrote:
> > On 4/23/20 11:01 AM, Geert Uytterhoeven wrote:
> >> Make the PL061 GPIO controller user-creatable, and allow the user to tie
> >> a newly created instance to a gpiochip on the host.
> >>
> >> To create a new GPIO controller, the QEMU command line must be augmented
> >> with:
> >>
> >> -device pl061,host=<gpiochip>
> >>
> >> with <gpiochip> the name or label of the gpiochip on the host.
> >>
> >> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >> --- a/hw/gpio/pl061.c
> >> +++ b/hw/gpio/pl061.c
> >> @@ -12,11 +12,14 @@
> >> #include "hw/arm/fdt.h"
> >> #include "hw/gpio/pl061.h"
> >> #include "hw/irq.h"
> >> +#include "hw/qdev-properties.h"
> >> #include "hw/sysbus.h"
> >> #include "migration/vmstate.h"
> >> +#include "qapi/error.h"
> >> #include "qemu/log.h"
> >> #include "qemu/module.h"
> >> #include "sysemu/device_tree.h"
> >> +#include "sysemu/gpiodev.h"
> >>
> >> //#define DEBUG_PL061 1
> >>
> >> @@ -41,6 +44,9 @@ static const uint8_t pl061_id_luminary[12] =
> >> typedef struct PL061State {
> >> SysBusDevice parent_obj;
> >>
> >> +#ifdef CONFIG_GPIODEV
> >> + char *host;
> >> +#endif
> >> MemoryRegion iomem;
> >> uint32_t locked;
> >> uint32_t data;
> >> @@ -370,10 +376,39 @@ static void pl061_init(Object *obj)
> >> qdev_init_gpio_out(dev, s->out, 8);
> >>
> >> +#ifdef CONFIG_GPIODEV
> >> +static Property pl061_properties[] = {
> >> + DEFINE_PROP_STRING("host", PL061State, host),
> >> + DEFINE_PROP_END_OF_LIST(),
> >> +};
> >> +
> >> +static void pl061_realize(DeviceState *dev, Error **errp)
> >> +{
> >> + PL061State *s = PL061(dev);
> >> +
> >> + if (!dev->opts) {
> >> + /* Not created by user */
> >> + return;
> >> + }
> >> +
> >> + if (!s->host) {
> >> + error_setg(errp, "'host' property is required");
> >> + return;
> >> + }
> >> +
> >> + qemu_gpiodev_add(dev, s->host, 8, errp);
> >> +}
> >> +#endif /* CONFIG_GPIODEV */
> >> +
> >> static void pl061_class_init(ObjectClass *klass, void *data)
> >> {
> >> DeviceClass *dc = DEVICE_CLASS(klass);
> >>
> >> +#ifdef CONFIG_GPIODEV
> >> + device_class_set_props(dc, pl061_properties);
> >> + dc->realize = pl061_realize;
> >> + dc->user_creatable = true;
> >> +#endif
> >> dc->vmsd = &vmstate_pl061;
> >> dc->reset = &pl061_reset;
> >> }
> >> diff --git a/qemu-options.hx b/qemu-options.hx
> >> index 292d4e7c0cef6097..182de7fb63923b38 100644
> >> --- a/qemu-options.hx
> >> +++ b/qemu-options.hx
> >> @@ -875,6 +875,15 @@ SRST
> >> ``-device isa-ipmi-bt,bmc=id[,ioport=val][,irq=val]``
> >> Like the KCS interface, but defines a BT interface. The default port
> >> is 0xe4 and the default interrupt is 5.
> >> +
> >> +#ifdef CONFIG_GPIODEV
> >> +``-device pl061,host=gpiochip``
> >> + Add a PL061 GPIO controller, and map its virtual GPIO lines to a GPIO
> >> + controller on the host.
> >> +
> >> + ``host=gpiochip``
> >> + The name or label of the GPIO controller on the host.
> >> +#endif
> >> ERST
> >>
> >> DEF("name", HAS_ARG, QEMU_OPTION_name,
> >>
> >
> > Instead of restricting this to the pl061, it would be cleaner you add a
> > GPIO_PLUGGABLE_INTERFACE (or GPIO_BINDABLE_INTERFACE or better name),
> > and have TYPE_PL061 implement it.
>
> IOW your backend should consume devices implementing this generic interface.
Thanks for the suggestion! I had a look into implementing this.
Please pardon my QEMU illiteracy, but I fail to see how adding an
interface, and letting frontends like pl061.c implement it, will help:
- The backend never has to call directly into the frontend: all
communication is done indirectly, using existing core qemu irq and
qdev gpio calls.
- The frontend has to call into the backend once, at initialization
time, to pass the host= parameter, and the number of GPIOs supported
by the frontend (through qemu_gpiodev_add()).
- Generalizing host= parsing in the backend would be nice, to avoid
duplicating this in each and every frontend, but AFAIU, an interface
cannot use device_class_set_props(), as InterfaceClass is not
derived from DeviceClass.
Note that when adding more features later (input support, and e.g.
pull-up/down support), the frontend will have to call into the backend
to change GPIO line configuration at runtime, but this is from frontend
to backend again, not the other way around.
I do agree that if we ever want to support multiple backends,
implementing that through an interface (for the backend, not for the
frontend) would be needed.
What am I missing?
Thanks again!
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
* Re: [PATCH v1] pinctrl: intel: Add Intel Emmitsburg pin controller support
From: Linus Walleij @ 2020-07-16 14:03 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Mika Westerberg, open list:GPIO SUBSYSTEM
In-Reply-To: <20200716124244.50797-1-andriy.shevchenko@linux.intel.com>
On Thu, Jul 16, 2020 at 2:42 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> This driver adds pinctrl/GPIO support for Intel Emmitsburg PCH. The
> GPIO controller is based on the next generation GPIO hardware but still
> compatible with the one supported by the Intel core pinctrl/GPIO driver.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
FWIW:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Obviously Mika's review is more important than mine anyway.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 20/25] pinctrl: pinctrl-rza1: Demote some kerneldoc headers and fix others
From: Linus Walleij @ 2020-07-16 13:57 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Lee Jones, Linux Kernel Mailing List, open list:GPIO SUBSYSTEM,
Geert Uytterhoeven, Jacopo Mondi, Linux-Renesas
In-Reply-To: <CAMuHMdWMUN8sU09J1eSsSJ9sXMhf10GUHeP47UDf6+yp8vnAnw@mail.gmail.com>
On Wed, Jul 15, 2020 at 9:30 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> i.e. will queue in sh-pfc-for-v5.9.
OK since Geert is queueing this I'll drop this patch from my tree.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 1/3] gpio: mxc: Support module build
From: Linus Walleij @ 2020-07-16 13:54 UTC (permalink / raw)
To: Anson Huang
Cc: Russell King, Shawn Guo, Sascha Hauer, Sascha Hauer,
Fabio Estevam, Catalin Marinas, Will Deacon, Bartosz Golaszewski,
oleksandr.suvorov@toradex.com, Adam Ford, Andreas Kemnade,
hverkuil-cisco@xs4all.nl, Bjorn Andersson, Leo Li, Vinod Koul,
Geert Uytterhoeven, Olof Johansson, Linux ARM,
linux-kernel@vger.kernel.org, open list:GPIO SUBSYSTEM,
dl-linux-imx
In-Reply-To: <DB3PR0402MB3916FB27846F462C2210C3BFF57E0@DB3PR0402MB3916.eurprd04.prod.outlook.com>
On Wed, Jul 15, 2020 at 4:44 AM Anson Huang <anson.huang@nxp.com> wrote:
> > Subject: RE: [PATCH 1/3] gpio: mxc: Support module build
> >
> > Hi, Linus
> >
> > > Subject: Re: [PATCH 1/3] gpio: mxc: Support module build
> > >
> > > On Wed, Jul 8, 2020 at 1:28 AM Anson Huang <Anson.Huang@nxp.com>
> > > wrote:
> > >
> > > > subsys_initcall(gpio_mxc_init);
> > > > +
> > > > +MODULE_AUTHOR("Shawn Guo <shawn.guo@linaro.org>");
> > > > +MODULE_DESCRIPTION("i.MX GPIO Driver"); MODULE_LICENSE("GPL");
> > >
> > > You are making this modualrizable but keeping the subsys_initcall(),
> > > which doesn't make very much sense. It is obviously not necessary to
> > > do this probe at subsys_initcall() time, right?
> > >
> >
> > If building it as module, the subsys_initcall() will be equal to module_init(), I
> > keep it unchanged is because I try to make it identical when built-in, since
> > most of the config will still have it built-in, except the Android GKI support.
> > Does it make sense?
> >
> > > Take this opportunity to convert the driver to use
> > > module_platform_driver() as well.
> >
> > If you think it has to be or it is better to use module_platform_driver(), I will do
> > it in V2.
>
> I tried to replace the subsys_initcall() with module_platform_driver(), but met issue
> about " register_syscore_ops(&mxc_gpio_syscore_ops);" which is called in gpio_mxc_init()
> function, this function should be called ONLY once, moving it to .probe function is NOT
> working, so we may need to keep the gpio_mxc_init(), that is another reason that we may
> need to keep subsys_initcall()?
This looks a bit dangerous to keep like this while allowing this
code to be used from a module.
What happens if you insmod and rmmod this a few times, really?
How is this tested?
This is not really modularized if that isn't working, just that modprobing
once works isn't real modularization IMO, it seems more like a
quick and dirty way to get Androids GKI somewhat working with the module
while not properly making the module a module.
You need input from the driver maintainers on how to handle this.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v8 1/7] pinctrl: mediatek: update pinmux definitions for mt6779
From: Linus Walleij @ 2020-07-16 13:49 UTC (permalink / raw)
To: Hanks Chen
Cc: Rob Herring, Matthias Brugger, Michael Turquette, Stephen Boyd,
Sean Wang, mtk01761, Andy Teng, open list:GPIO SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, moderated list:ARM/Mediatek SoC support,
linux-kernel@vger.kernel.org, wsd_upstream, CC Hwang, Loda Chou,
Mars Cheng
In-Reply-To: <1594718402-20813-2-git-send-email-hanks.chen@mediatek.com>
On Tue, Jul 14, 2020 at 11:20 AM Hanks Chen <hanks.chen@mediatek.com> wrote:
> Add devicetree bindings for Mediatek mt6779 SoC Pin Controller.
>
> Acked-by: Sean Wang <sean.wang@kernel.org>
> Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
> Signed-off-by: Andy Teng <andy.teng@mediatek.com>
> Signed-off-by: Hanks Chen <hanks.chen@mediatek.com>
Sorry for responding to old patches :/
This and the rest of the pinctrl patches are now applied
to the pinctrl tree for v5.9.
The DTS and clock patches need to be applied elsewhere.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v3] pinctrl: qcom: Handle broken/missing PDC dual edge IRQs on sc7180
From: Linus Walleij @ 2020-07-16 13:42 UTC (permalink / raw)
To: Douglas Anderson, Bjorn Andersson
Cc: Rajendra Nayak, Maulik Shah, Marc Zyngier, Lina Iyer,
Cheng-Yi Chiang, Stephen Boyd, Andy Gross, MSM,
open list:GPIO SUBSYSTEM, linux-kernel@vger.kernel.org
In-Reply-To: <20200714080254.v3.1.Ie0d730120b232a86a4eac1e2909bcbec844d1766@changeid>
On Tue, Jul 14, 2020 at 5:04 PM Douglas Anderson <dianders@chromium.org> wrote:
> Depending on how you look at it, you can either say that:
> a) There is a PDC hardware issue (with the specific IP rev that exists
> on sc7180) that causes the PDC not to work properly when configured
> to handle dual edges.
> b) The dual edge feature of the PDC hardware was only added in later
> HW revisions and thus isn't in all hardware.
>
> Regardless of how you look at it, let's work around the lack of dual
> edge support by only ever letting our parent see requests for single
> edge interrupts on affected hardware.
>
> NOTE: it's possible that a driver requesting a dual edge interrupt
> might get several edges coalesced into a single IRQ. For instance if
> a line starts low and then goes high and low again, the driver that
> requested the IRQ is not guaranteed to be called twice. However, it
> is guaranteed that once the driver's interrupt handler starts running
> its first instruction that any new edges coming in will cause the
> interrupt to fire again. This is relatively commonplace for dual-edge
> gpio interrupts (many gpio controllers require software to emulate
> dual edge with single edge) so client drivers should be setup to
> handle it.
>
> Fixes: e35a6ae0eb3a ("pinctrl/msm: Setup GPIO chip in hierarchy")
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> As far as I can tell everything here should work and the limited
> testing I'm able to give it shows that, in fact, I can detect both
> edges.
>
> I specifically left off Reviewed-by and Tested-by tags from v2 becuase
> I felt that the implementation had changed just enough to invalidate
> previous reviews / testing. Hopefully it's not too much of a hassle
> for folks to re-review and re-test.
>
> Changes in v3:
> - Rate limit the warning.
Tentatively applied this to the fixes branch in the pinctrl tree
so we get some linux-next coverage.
Would be nice to get Bjorn's ACK on it as well!
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 16/25] arch: arm: mach-at91: pm: Move prototypes to mutually included header
From: Lee Jones @ 2020-07-16 13:42 UTC (permalink / raw)
To: Alexandre Belloni
Cc: linus.walleij, linux-kernel, linux-gpio, Russell King,
Nicolas Ferre, Ludovic Desroches
In-Reply-To: <20200713200244.GA23553@piout.net>
On Mon, 13 Jul 2020, Alexandre Belloni wrote:
> Hi,
>
> On 13/07/2020 15:49:21+0100, Lee Jones wrote:
> > Both the caller and the supplier's source file should have access to
> > the include file containing the prototypes.
> >
> > Fixes the following W=1 kernel build warning(s):
> >
> > drivers/pinctrl/pinctrl-at91.c:1637:6: warning: no previous prototype for ‘at91_pinctrl_gpio_suspend’ [-Wmissing-prototypes]
> > 1637 | void at91_pinctrl_gpio_suspend(void)
> > | ^~~~~~~~~~~~~~~~~~~~~~~~~
> > drivers/pinctrl/pinctrl-at91.c:1661:6: warning: no previous prototype for ‘at91_pinctrl_gpio_resume’ [-Wmissing-prototypes]
> > 1661 | void at91_pinctrl_gpio_resume(void)
> > | ^~~~~~~~~~~~~~~~~~~~~~~~
> >
> > Cc: Russell King <linux@armlinux.org.uk>
> > Cc: Nicolas Ferre <nicolas.ferre@microchip.com>
> > Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
> > Cc: Ludovic Desroches <ludovic.desroches@microchip.com>
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> > arch/arm/mach-at91/pm.c | 17 ++++++-----------
> > drivers/pinctrl/pinctrl-at91.c | 1 +
> > include/linux/platform_data/atmel.h | 5 +++++
> > 3 files changed, 12 insertions(+), 11 deletions(-)
> >
> > diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> > index 074bde64064e4..59f2a2d6fbbb8 100644
> > --- a/arch/arm/mach-at91/pm.c
> > +++ b/arch/arm/mach-at91/pm.c
> > @@ -25,17 +25,6 @@
> > #include "generic.h"
> > #include "pm.h"
> >
> > -/*
> > - * FIXME: this is needed to communicate between the pinctrl driver and
> > - * the PM implementation in the machine. Possibly part of the PM
> > - * implementation should be moved down into the pinctrl driver and get
> > - * called as part of the generic suspend/resume path.
> > - */
> > -#ifdef CONFIG_PINCTRL_AT91
> > -extern void at91_pinctrl_gpio_suspend(void);
> > -extern void at91_pinctrl_gpio_resume(void);
> > -#endif
> > -
> > struct at91_soc_pm {
> > int (*config_shdwc_ws)(void __iomem *shdwc, u32 *mode, u32 *polarity);
> > int (*config_pmc_ws)(void __iomem *pmc, u32 mode, u32 polarity);
> > @@ -325,6 +314,12 @@ static void at91_pm_suspend(suspend_state_t state)
> > static int at91_pm_enter(suspend_state_t state)
> > {
> > #ifdef CONFIG_PINCTRL_AT91
> > + /*
> > + * FIXME: this is needed to communicate between the pinctrl driver and
> > + * the PM implementation in the machine. Possibly part of the PM
> > + * implementation should be moved down into the pinctrl driver and get
> > + * called as part of the generic suspend/resume path.
> > + */
> > at91_pinctrl_gpio_suspend();
> > #endif
> >
> > diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> > index 9c52130876597..37997e5ab0538 100644
> > --- a/drivers/pinctrl/pinctrl-at91.c
> > +++ b/drivers/pinctrl/pinctrl-at91.c
> > @@ -22,6 +22,7 @@
> > #include <linux/pinctrl/pinmux.h>
> > /* Since we request GPIOs from ourself */
> > #include <linux/pinctrl/consumer.h>
> > +#include <linux/platform_data/atmel.h>
> >
> > #include "pinctrl-at91.h"
> > #include "core.h"
> > diff --git a/include/linux/platform_data/atmel.h b/include/linux/platform_data/atmel.h
> > index 99e6069c5fd89..666ef482ea8c0 100644
> > --- a/include/linux/platform_data/atmel.h
> > +++ b/include/linux/platform_data/atmel.h
>
> The plan is to get rid of that file so you should probably find a better
> location.
Suggestions welcome.
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH RESEND v2] dt-bindings: pinctrl: Convert ingenic,pinctrl.txt to YAML
From: Linus Walleij @ 2020-07-16 13:40 UTC (permalink / raw)
To: Rob Herring
Cc: Paul Cercueil, od, open list:GPIO SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-kernel@vger.kernel.org
In-Reply-To: <CAL_Jsq+nHZsbOMPpXC7NWp1etgVL57Q+o=gr6BJ6ijAq1pLJUw@mail.gmail.com>
On Tue, Jul 14, 2020 at 4:05 PM Rob Herring <robh@kernel.org> wrote:
> On Mon, Jul 13, 2020 at 9:36 AM Paul Cercueil <paul@crapouillou.net> wrote:
> > >> Notes:
> > >> v2: - Use 'pinctrl' instead of 'pin-controller' as the node name
> > >> - remove 'additionalProperties: false' since we will have
> > >> pin conf nodes
> > >
> > > What do those look like? They need to be described, but that can be a
> > > follow-up.
> >
> > These are generic conf nodes that are handled by the pinctrl core.
>
> No such thing. There's a set of common properties, but that is all.
> You still need to document which properties apply because it is
> doubtful they all do.
Paul can you make a follow-up patch to fix this?
Yours,
Linus Walleij
^ permalink raw reply
* Re: [RFC v2 GPIO lines [was: GPIO User I/O]
From: Linus Walleij @ 2020-07-16 13:38 UTC (permalink / raw)
To: Rodolfo Giometti
Cc: Geert Uytterhoeven, Geert Uytterhoeven, open list:GPIO SUBSYSTEM,
Bartosz Golaszewski, Rob Herring,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
In-Reply-To: <cb6e208b-446e-eba4-b324-d88aec94a69b@enneenne.com>
On Tue, Jul 14, 2020 at 4:01 PM Rodolfo Giometti <giometti@enneenne.com> wrote:
> I see... however attached is a new version of my proposal patch
I looked a bit at this!
IIUC the idea is a "new" sysfs interface that does not require the exporting
etc used by the current "old" sysfs interface. Instead of poking around in
sysfs to export lines we do that from the device tree.
It also does not use any global GPIO numbers which would be my other
main concern.
I must admit that it has some elegance to it. Especially when it comes
to scripting.
The problem I see is that lines are left in whatever state they were in
if a script crashes, so there is no "return to the initial value" that was
there when the GPIOs were picked from the device tree. This makes
this a bit fragile.
Also users regularly need to listen to events. This interface can and
should never support that, for this one must use the character device,
which will of course not work in parallel with using this sysfs ABI.
And the day someone wants that we simply have to say no. There
is no way to hold states for event handling in a stateless ABI.
Well of course they can poll for a line to change, but that is not
proper event handling that reacts to an interrupt.
So while this is much more elegant the old sysfs ABI, and certainly
better for scripting, it still suffers from some conflicts with
the character device, and there is a risk to make users dissatisfied
when they want to e.g. script event handlers.
What are your thoughts on this?
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 2/7] dt-bindings: pinctrl: add bindings for MediaTek MT6779 SoC
From: Linus Walleij @ 2020-07-16 13:26 UTC (permalink / raw)
To: Hanks Chen
Cc: Rob Herring, Matthias Brugger, Michael Turquette, Stephen Boyd,
Sean Wang, mtk01761, Andy Teng, open list:GPIO SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, moderated list:ARM/Mediatek SoC support,
linux-kernel@vger.kernel.org, wsd_upstream, CC Hwang, Loda Chou
In-Reply-To: <1594717479-8160-3-git-send-email-hanks.chen@mediatek.com>
On Tue, Jul 14, 2020 at 11:04 AM Hanks Chen <hanks.chen@mediatek.com> wrote:
> From: Andy Teng <andy.teng@mediatek.com>
>
> Add devicetree bindings for MediaTek MT6779 pinctrl driver.
>
> Signed-off-by: Andy Teng <andy.teng@mediatek.com>
(...)
Please make an attempt to reuse the generic schemas in
Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml
Documentation/devicetree/bindings/pinctrl/pinmux-node.yaml
See how other bindings reuse them, e.g.:
qcom,ipq6018-pinctrl.yaml
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v3 3/5] pinctrl: qcom: Use return value from irq_set_wake call
From: Linus Walleij @ 2020-07-16 13:18 UTC (permalink / raw)
To: Maulik Shah
Cc: Bjorn Andersson, Marc Zyngier, Stephen Boyd, Evan Green,
Matthias Kaehlcke, linux-kernel@vger.kernel.org, MSM,
open list:GPIO SUBSYSTEM, Andy Gross, Thomas Gleixner,
Jason Cooper, Doug Anderson, Rajendra Nayak, Lina Iyer,
open list:GPIO SUBSYSTEM <linux-gpio@vger.kernel.org>, Andy Gross <agross@kernel.org>, Thomas Gleixner <tglx@linutronix.de>, Jason Cooper <jason@lakedaemon.net>, Doug Anderson <dianders@chromium.org>, Rajendra Nayak <rnayak@codeaurora.org>, Lina Iyer <ilina@codeaurora.org>,
In-Reply-To: <1592818308-23001-4-git-send-email-mkshah@codeaurora.org>
On Mon, Jun 22, 2020 at 11:32 AM Maulik Shah <mkshah@codeaurora.org> wrote:
> msmgpio irqchip is not using return value of irq_set_wake call.
> Start using it.
>
> Fixes: e35a6ae0eb3a ("pinctrl/msm: Setup GPIO chip in hierarchy")
> Signed-off-by: Maulik Shah <mkshah@codeaurora.org>
Is this something that's causing regressions so I should apply it for
fixes, or is it fine to keep this with the rest of the series for v5.9?
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 16/25] arch: arm: mach-at91: pm: Move prototypes to mutually included header
From: Linus Walleij @ 2020-07-16 13:14 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Lee Jones, linux-kernel@vger.kernel.org, open list:GPIO SUBSYSTEM,
Russell King, Nicolas Ferre, Ludovic Desroches
In-Reply-To: <20200713200244.GA23553@piout.net>
On Mon, Jul 13, 2020 at 10:02 PM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
> On 13/07/2020 15:49:21+0100, Lee Jones wrote:
> > diff --git a/include/linux/platform_data/atmel.h b/include/linux/platform_data/atmel.h
> > index 99e6069c5fd89..666ef482ea8c0 100644
> > --- a/include/linux/platform_data/atmel.h
> > +++ b/include/linux/platform_data/atmel.h
>
> The plan is to get rid of that file so you should probably find a better
> location.
OK I dropped this one patch from the set for now.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v1] gpio: mmio: replace open-coded for_each_set_bit()
From: Linus Walleij @ 2020-07-16 13:08 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Bartosz Golaszewski, open list:GPIO SUBSYSTEM
In-Reply-To: <20200713154429.23662-1-andriy.shevchenko@linux.intel.com>
On Mon, Jul 13, 2020 at 5:44 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> Use for_each_set_bit() instead of open-coding it to simplify the code.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Excellent, patch applied!
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 00/25] Rid W=1 warnings in Pinctrl
From: Linus Walleij @ 2020-07-16 13:05 UTC (permalink / raw)
To: Lee Jones; +Cc: linux-kernel@vger.kernel.org, open list:GPIO SUBSYSTEM
In-Reply-To: <20200713144930.1034632-1-lee.jones@linaro.org>
On Mon, Jul 13, 2020 at 4:49 PM Lee Jones <lee.jones@linaro.org> wrote:
> pinctrl: pinctrl-single: Fix struct/function documentation blocks
This patch didn't apply to v5.8-rc1 so I applied that one separately
after merging in all the other patches from a branch.
Seems to work! So all applied.
Also THANKS for doing this!!
Yours,
Linus Walleij
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox