From: Qianggui Song <qianggui.song@amlogic.com>
To: Neil Armstrong <narmstrong@baylibre.com>,
Linus Walleij <linus.walleij@linaro.org>,
<linux-gpio@vger.kernel.org>
Cc: Jerome Brunet <jbrunet@baylibre.com>,
Kevin Hilman <khilman@baylibre.com>,
Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
Carlo Caione <carlo@caione.org>, Rob Herring <robh+dt@kernel.org>,
Xingyu Chen <xingyu.chen@amlogic.com>,
Jianxin Pan <jianxin.pan@amlogic.com>,
Hanjie Lin <hanjie.lin@amlogic.com>,
Mark Rutland <mark.rutland@arm.com>,
<linux-arm-kernel@lists.infradead.org>,
<linux-amlogic@lists.infradead.org>,
<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 2/3] pinctrl: meson-a1: add pinctrl driver for Meson A1 Soc
Date: Thu, 10 Oct 2019 20:02:42 +0800 [thread overview]
Message-ID: <a41f0685-a42c-b21c-d0be-e0e1c3ae7c8f@amlogic.com> (raw)
In-Reply-To: <cca24aa5-07dc-f2d6-885a-09bc8e20b3b6@baylibre.com>
Hi,Neil
On 2019/10/8 21:07, Neil Armstrong wrote:
> Hi,
>
> On 08/10/2019 13:09, Qianggui Song wrote:
>> Add pinctrl driver for Meson A1 Soc which share the same register layout of
>> pinmux with previous Meson-G12A, however there is difference for gpio
>> and pin config register in A1.The main difference is that registers before A1
>> are grouped by function while those of A1 are by bank. The new register layout
>> is as below:
>>
>> /* first bank */ /* addr */
>> - P_PADCTRL_GPIOP_I base + 0x00 << 2
>> - P_PADCTRL_GPIOP_O base + 0x01 << 2
>> - P_PADCTRL_GPIOP_OEN base + 0x02 << 2
>> - P_PADCTRL_GPIOP_PULL_EN base + 0x03 << 2
>> - P_PADCTRL_GPIOP_PULL_UP base + 0x04 << 2
>> - P_PADCTRL_GPIOP_DS base + 0x05 << 2
>>
>> /* second bank */
>> - P_PADCTRL_GPIOB_I base + 0x10 << 2
>> - P_PADCTRL_GPIOB_O base + 0x11 << 2
>> - P_PADCTRL_GPIOB_OEN base + 0x12 << 2
>> - P_PADCTRL_GPIOB_PULL_EN base + 0x13 << 2
>> - P_PADCTRL_GPIOB_PULL_UP base + 0x14 << 2
>> - P_PADCTRL_GPIOB_DS base + 0x15 << 2
>>
>> Each bank contains at least 6 registers to be configured, if one bank has
>> more than 16 gpios, an extra P_PADCTRL_GPIO[X]_DS_EXT is included. Between
>> two adjacent P_PADCTRL_GPIO[X]_I, there is an offset 0x10, that is to say,
>> for third bank, the offsets will be 0x20,0x21,0x22,0x23,0x24,0x25 according
>> to above register layout.For privous chips, registers are grouped
>> according to their functions while registers of A1 are according to bank.
>>
>> Current Meson pinctrl driver can cover such change by using base address of
>> GPIO as that of drive-strength. While simply giving reg_ds = reg_pullen
>> make wrong value to reg_ds for Socs that do not support drive-strength like
>> AXG.To make things simple, add an extra dt parser function for a1 or later chip
>> and remain the old dt parser function for old Socs.
>>
>> Also note that there is no AO bank in A1.
>>
>> Signed-off-by: Qianggui Song <qianggui.song@amlogic.com>
>> ---
>> drivers/pinctrl/meson/Kconfig | 6 +
>> drivers/pinctrl/meson/Makefile | 1 +
>> drivers/pinctrl/meson/pinctrl-meson-a1.c | 942 +++++++++++++++++++++++++++++++
>> drivers/pinctrl/meson/pinctrl-meson.c | 16 +-
>> drivers/pinctrl/meson/pinctrl-meson.h | 5 +
>> 5 files changed, 969 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/pinctrl/meson/pinctrl-meson-a1.c
>>
>
> [...]
>
>> --- a/drivers/pinctrl/meson/pinctrl-meson.c
>> +++ b/drivers/pinctrl/meson/pinctrl-meson.c
>> @@ -695,6 +695,17 @@ static int meson_pinctrl_parse_dt(struct meson_pinctrl *pc,
>> return 0;
>> }
>>
>> +int meson_pinctrl_parse_dt_extra(struct meson_pinctrl *pc,
>> + struct device_node *node)
>> +{
>> + int ret;
>> +
>> + ret = meson_pinctrl_parse_dt(pc, node);
>> + pc->reg_ds = pc->reg_pullen;
>> +
>> + return ret;
>> +}
>> +
>> int meson_pinctrl_probe(struct platform_device *pdev)
>> {
>> struct device *dev = &pdev->dev;
>> @@ -708,7 +719,10 @@ int meson_pinctrl_probe(struct platform_device *pdev)
>> pc->dev = dev;
>> pc->data = (struct meson_pinctrl_data *) of_device_get_match_data(dev);
>>
>> - ret = meson_pinctrl_parse_dt(pc, dev->of_node);
>> + if (pc->data->parse_dt)
>> + ret = pc->data->parse_dt(pc, dev->of_node);
>> + else
>> + ret = meson_pinctrl_parse_dt(pc, dev->of_node);
>> if (ret)
>> return ret;
>>
>> diff --git a/drivers/pinctrl/meson/pinctrl-meson.h b/drivers/pinctrl/meson/pinctrl-meson.h
>> index c696f3241a36..ca29efd90aac 100644
>> --- a/drivers/pinctrl/meson/pinctrl-meson.h
>> +++ b/drivers/pinctrl/meson/pinctrl-meson.h
>> @@ -11,6 +11,7 @@
>> #include <linux/regmap.h>
>> #include <linux/types.h>
>>
>> +struct meson_pinctrl;
>> /**
>> * struct meson_pmx_group - a pinmux group
>> *
>> @@ -114,6 +115,7 @@ struct meson_pinctrl_data {
>> unsigned int num_banks;
>> const struct pinmux_ops *pmx_ops;
>> void *pmx_data;
>> + int (*parse_dt) (struct meson_pinctrl *pc, struct device_node *node);
>> };
>>
>> struct meson_pinctrl {
>> @@ -171,3 +173,6 @@ int meson_pmx_get_groups(struct pinctrl_dev *pcdev,
>>
>> /* Common probe function */
>> int meson_pinctrl_probe(struct platform_device *pdev);
>> +/* Extra dt parser function for register layout grouped by bank */
>> +int meson_pinctrl_parse_dt_extra(struct meson_pinctrl *pc,
>> + struct device_node *node);
>>
>
> I think you should add this parse_dt callback in a separate patch.
>
> Neil
>
> .
>
OK, will do it in the next patch set.
WARNING: multiple messages have this Message-ID (diff)
From: Qianggui Song <qianggui.song@amlogic.com>
To: Neil Armstrong <narmstrong@baylibre.com>,
Linus Walleij <linus.walleij@linaro.org>,
<linux-gpio@vger.kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>,
Hanjie Lin <hanjie.lin@amlogic.com>,
Jianxin Pan <jianxin.pan@amlogic.com>,
Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
Kevin Hilman <khilman@baylibre.com>,
linux-kernel@vger.kernel.org, Rob Herring <robh+dt@kernel.org>,
linux-arm-kernel@lists.infradead.org,
Carlo Caione <carlo@caione.org>,
linux-amlogic@lists.infradead.org,
Xingyu Chen <xingyu.chen@amlogic.com>,
Jerome Brunet <jbrunet@baylibre.com>
Subject: Re: [PATCH v2 2/3] pinctrl: meson-a1: add pinctrl driver for Meson A1 Soc
Date: Thu, 10 Oct 2019 20:02:42 +0800 [thread overview]
Message-ID: <a41f0685-a42c-b21c-d0be-e0e1c3ae7c8f@amlogic.com> (raw)
In-Reply-To: <cca24aa5-07dc-f2d6-885a-09bc8e20b3b6@baylibre.com>
Hi,Neil
On 2019/10/8 21:07, Neil Armstrong wrote:
> Hi,
>
> On 08/10/2019 13:09, Qianggui Song wrote:
>> Add pinctrl driver for Meson A1 Soc which share the same register layout of
>> pinmux with previous Meson-G12A, however there is difference for gpio
>> and pin config register in A1.The main difference is that registers before A1
>> are grouped by function while those of A1 are by bank. The new register layout
>> is as below:
>>
>> /* first bank */ /* addr */
>> - P_PADCTRL_GPIOP_I base + 0x00 << 2
>> - P_PADCTRL_GPIOP_O base + 0x01 << 2
>> - P_PADCTRL_GPIOP_OEN base + 0x02 << 2
>> - P_PADCTRL_GPIOP_PULL_EN base + 0x03 << 2
>> - P_PADCTRL_GPIOP_PULL_UP base + 0x04 << 2
>> - P_PADCTRL_GPIOP_DS base + 0x05 << 2
>>
>> /* second bank */
>> - P_PADCTRL_GPIOB_I base + 0x10 << 2
>> - P_PADCTRL_GPIOB_O base + 0x11 << 2
>> - P_PADCTRL_GPIOB_OEN base + 0x12 << 2
>> - P_PADCTRL_GPIOB_PULL_EN base + 0x13 << 2
>> - P_PADCTRL_GPIOB_PULL_UP base + 0x14 << 2
>> - P_PADCTRL_GPIOB_DS base + 0x15 << 2
>>
>> Each bank contains at least 6 registers to be configured, if one bank has
>> more than 16 gpios, an extra P_PADCTRL_GPIO[X]_DS_EXT is included. Between
>> two adjacent P_PADCTRL_GPIO[X]_I, there is an offset 0x10, that is to say,
>> for third bank, the offsets will be 0x20,0x21,0x22,0x23,0x24,0x25 according
>> to above register layout.For privous chips, registers are grouped
>> according to their functions while registers of A1 are according to bank.
>>
>> Current Meson pinctrl driver can cover such change by using base address of
>> GPIO as that of drive-strength. While simply giving reg_ds = reg_pullen
>> make wrong value to reg_ds for Socs that do not support drive-strength like
>> AXG.To make things simple, add an extra dt parser function for a1 or later chip
>> and remain the old dt parser function for old Socs.
>>
>> Also note that there is no AO bank in A1.
>>
>> Signed-off-by: Qianggui Song <qianggui.song@amlogic.com>
>> ---
>> drivers/pinctrl/meson/Kconfig | 6 +
>> drivers/pinctrl/meson/Makefile | 1 +
>> drivers/pinctrl/meson/pinctrl-meson-a1.c | 942 +++++++++++++++++++++++++++++++
>> drivers/pinctrl/meson/pinctrl-meson.c | 16 +-
>> drivers/pinctrl/meson/pinctrl-meson.h | 5 +
>> 5 files changed, 969 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/pinctrl/meson/pinctrl-meson-a1.c
>>
>
> [...]
>
>> --- a/drivers/pinctrl/meson/pinctrl-meson.c
>> +++ b/drivers/pinctrl/meson/pinctrl-meson.c
>> @@ -695,6 +695,17 @@ static int meson_pinctrl_parse_dt(struct meson_pinctrl *pc,
>> return 0;
>> }
>>
>> +int meson_pinctrl_parse_dt_extra(struct meson_pinctrl *pc,
>> + struct device_node *node)
>> +{
>> + int ret;
>> +
>> + ret = meson_pinctrl_parse_dt(pc, node);
>> + pc->reg_ds = pc->reg_pullen;
>> +
>> + return ret;
>> +}
>> +
>> int meson_pinctrl_probe(struct platform_device *pdev)
>> {
>> struct device *dev = &pdev->dev;
>> @@ -708,7 +719,10 @@ int meson_pinctrl_probe(struct platform_device *pdev)
>> pc->dev = dev;
>> pc->data = (struct meson_pinctrl_data *) of_device_get_match_data(dev);
>>
>> - ret = meson_pinctrl_parse_dt(pc, dev->of_node);
>> + if (pc->data->parse_dt)
>> + ret = pc->data->parse_dt(pc, dev->of_node);
>> + else
>> + ret = meson_pinctrl_parse_dt(pc, dev->of_node);
>> if (ret)
>> return ret;
>>
>> diff --git a/drivers/pinctrl/meson/pinctrl-meson.h b/drivers/pinctrl/meson/pinctrl-meson.h
>> index c696f3241a36..ca29efd90aac 100644
>> --- a/drivers/pinctrl/meson/pinctrl-meson.h
>> +++ b/drivers/pinctrl/meson/pinctrl-meson.h
>> @@ -11,6 +11,7 @@
>> #include <linux/regmap.h>
>> #include <linux/types.h>
>>
>> +struct meson_pinctrl;
>> /**
>> * struct meson_pmx_group - a pinmux group
>> *
>> @@ -114,6 +115,7 @@ struct meson_pinctrl_data {
>> unsigned int num_banks;
>> const struct pinmux_ops *pmx_ops;
>> void *pmx_data;
>> + int (*parse_dt) (struct meson_pinctrl *pc, struct device_node *node);
>> };
>>
>> struct meson_pinctrl {
>> @@ -171,3 +173,6 @@ int meson_pmx_get_groups(struct pinctrl_dev *pcdev,
>>
>> /* Common probe function */
>> int meson_pinctrl_probe(struct platform_device *pdev);
>> +/* Extra dt parser function for register layout grouped by bank */
>> +int meson_pinctrl_parse_dt_extra(struct meson_pinctrl *pc,
>> + struct device_node *node);
>>
>
> I think you should add this parse_dt callback in a separate patch.
>
> Neil
>
> .
>
OK, will do it in the next patch set.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: Qianggui Song <qianggui.song@amlogic.com>
To: Neil Armstrong <narmstrong@baylibre.com>,
Linus Walleij <linus.walleij@linaro.org>,
<linux-gpio@vger.kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>,
Hanjie Lin <hanjie.lin@amlogic.com>,
Jianxin Pan <jianxin.pan@amlogic.com>,
Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
Kevin Hilman <khilman@baylibre.com>,
linux-kernel@vger.kernel.org, Rob Herring <robh+dt@kernel.org>,
linux-arm-kernel@lists.infradead.org,
Carlo Caione <carlo@caione.org>,
linux-amlogic@lists.infradead.org,
Xingyu Chen <xingyu.chen@amlogic.com>,
Jerome Brunet <jbrunet@baylibre.com>
Subject: Re: [PATCH v2 2/3] pinctrl: meson-a1: add pinctrl driver for Meson A1 Soc
Date: Thu, 10 Oct 2019 20:02:42 +0800 [thread overview]
Message-ID: <a41f0685-a42c-b21c-d0be-e0e1c3ae7c8f@amlogic.com> (raw)
In-Reply-To: <cca24aa5-07dc-f2d6-885a-09bc8e20b3b6@baylibre.com>
Hi,Neil
On 2019/10/8 21:07, Neil Armstrong wrote:
> Hi,
>
> On 08/10/2019 13:09, Qianggui Song wrote:
>> Add pinctrl driver for Meson A1 Soc which share the same register layout of
>> pinmux with previous Meson-G12A, however there is difference for gpio
>> and pin config register in A1.The main difference is that registers before A1
>> are grouped by function while those of A1 are by bank. The new register layout
>> is as below:
>>
>> /* first bank */ /* addr */
>> - P_PADCTRL_GPIOP_I base + 0x00 << 2
>> - P_PADCTRL_GPIOP_O base + 0x01 << 2
>> - P_PADCTRL_GPIOP_OEN base + 0x02 << 2
>> - P_PADCTRL_GPIOP_PULL_EN base + 0x03 << 2
>> - P_PADCTRL_GPIOP_PULL_UP base + 0x04 << 2
>> - P_PADCTRL_GPIOP_DS base + 0x05 << 2
>>
>> /* second bank */
>> - P_PADCTRL_GPIOB_I base + 0x10 << 2
>> - P_PADCTRL_GPIOB_O base + 0x11 << 2
>> - P_PADCTRL_GPIOB_OEN base + 0x12 << 2
>> - P_PADCTRL_GPIOB_PULL_EN base + 0x13 << 2
>> - P_PADCTRL_GPIOB_PULL_UP base + 0x14 << 2
>> - P_PADCTRL_GPIOB_DS base + 0x15 << 2
>>
>> Each bank contains at least 6 registers to be configured, if one bank has
>> more than 16 gpios, an extra P_PADCTRL_GPIO[X]_DS_EXT is included. Between
>> two adjacent P_PADCTRL_GPIO[X]_I, there is an offset 0x10, that is to say,
>> for third bank, the offsets will be 0x20,0x21,0x22,0x23,0x24,0x25 according
>> to above register layout.For privous chips, registers are grouped
>> according to their functions while registers of A1 are according to bank.
>>
>> Current Meson pinctrl driver can cover such change by using base address of
>> GPIO as that of drive-strength. While simply giving reg_ds = reg_pullen
>> make wrong value to reg_ds for Socs that do not support drive-strength like
>> AXG.To make things simple, add an extra dt parser function for a1 or later chip
>> and remain the old dt parser function for old Socs.
>>
>> Also note that there is no AO bank in A1.
>>
>> Signed-off-by: Qianggui Song <qianggui.song@amlogic.com>
>> ---
>> drivers/pinctrl/meson/Kconfig | 6 +
>> drivers/pinctrl/meson/Makefile | 1 +
>> drivers/pinctrl/meson/pinctrl-meson-a1.c | 942 +++++++++++++++++++++++++++++++
>> drivers/pinctrl/meson/pinctrl-meson.c | 16 +-
>> drivers/pinctrl/meson/pinctrl-meson.h | 5 +
>> 5 files changed, 969 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/pinctrl/meson/pinctrl-meson-a1.c
>>
>
> [...]
>
>> --- a/drivers/pinctrl/meson/pinctrl-meson.c
>> +++ b/drivers/pinctrl/meson/pinctrl-meson.c
>> @@ -695,6 +695,17 @@ static int meson_pinctrl_parse_dt(struct meson_pinctrl *pc,
>> return 0;
>> }
>>
>> +int meson_pinctrl_parse_dt_extra(struct meson_pinctrl *pc,
>> + struct device_node *node)
>> +{
>> + int ret;
>> +
>> + ret = meson_pinctrl_parse_dt(pc, node);
>> + pc->reg_ds = pc->reg_pullen;
>> +
>> + return ret;
>> +}
>> +
>> int meson_pinctrl_probe(struct platform_device *pdev)
>> {
>> struct device *dev = &pdev->dev;
>> @@ -708,7 +719,10 @@ int meson_pinctrl_probe(struct platform_device *pdev)
>> pc->dev = dev;
>> pc->data = (struct meson_pinctrl_data *) of_device_get_match_data(dev);
>>
>> - ret = meson_pinctrl_parse_dt(pc, dev->of_node);
>> + if (pc->data->parse_dt)
>> + ret = pc->data->parse_dt(pc, dev->of_node);
>> + else
>> + ret = meson_pinctrl_parse_dt(pc, dev->of_node);
>> if (ret)
>> return ret;
>>
>> diff --git a/drivers/pinctrl/meson/pinctrl-meson.h b/drivers/pinctrl/meson/pinctrl-meson.h
>> index c696f3241a36..ca29efd90aac 100644
>> --- a/drivers/pinctrl/meson/pinctrl-meson.h
>> +++ b/drivers/pinctrl/meson/pinctrl-meson.h
>> @@ -11,6 +11,7 @@
>> #include <linux/regmap.h>
>> #include <linux/types.h>
>>
>> +struct meson_pinctrl;
>> /**
>> * struct meson_pmx_group - a pinmux group
>> *
>> @@ -114,6 +115,7 @@ struct meson_pinctrl_data {
>> unsigned int num_banks;
>> const struct pinmux_ops *pmx_ops;
>> void *pmx_data;
>> + int (*parse_dt) (struct meson_pinctrl *pc, struct device_node *node);
>> };
>>
>> struct meson_pinctrl {
>> @@ -171,3 +173,6 @@ int meson_pmx_get_groups(struct pinctrl_dev *pcdev,
>>
>> /* Common probe function */
>> int meson_pinctrl_probe(struct platform_device *pdev);
>> +/* Extra dt parser function for register layout grouped by bank */
>> +int meson_pinctrl_parse_dt_extra(struct meson_pinctrl *pc,
>> + struct device_node *node);
>>
>
> I think you should add this parse_dt callback in a separate patch.
>
> Neil
>
> .
>
OK, will do it in the next patch set.
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
next prev parent reply other threads:[~2019-10-10 12:02 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-08 11:09 [PATCH v2 0/3] pinctrl: meson-a1: add pinctrl driver Qianggui Song
2019-10-08 11:09 ` Qianggui Song
2019-10-08 11:09 ` Qianggui Song
2019-10-08 11:09 ` Qianggui Song
2019-10-08 11:09 ` [PATCH v2 1/3] pinctrl: add compatible for Amlogic Meson A1 pin controller Qianggui Song
2019-10-08 11:09 ` Qianggui Song
2019-10-08 11:09 ` Qianggui Song
2019-10-08 11:09 ` Qianggui Song
2019-10-08 11:09 ` [PATCH v2 2/3] pinctrl: meson-a1: add pinctrl driver for Meson A1 Soc Qianggui Song
2019-10-08 11:09 ` Qianggui Song
2019-10-08 11:09 ` Qianggui Song
2019-10-08 13:07 ` Neil Armstrong
2019-10-08 13:07 ` Neil Armstrong
2019-10-08 13:07 ` Neil Armstrong
2019-10-10 12:02 ` Qianggui Song [this message]
2019-10-10 12:02 ` Qianggui Song
2019-10-10 12:02 ` Qianggui Song
2019-10-08 11:09 ` [PATCH v2 3/3] arm64: dts: meson: a1: add pinctrl controller support Qianggui Song
2019-10-08 11:09 ` Qianggui Song
2019-10-08 11:09 ` Qianggui Song
2019-10-08 11:09 ` Qianggui Song
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a41f0685-a42c-b21c-d0be-e0e1c3ae7c8f@amlogic.com \
--to=qianggui.song@amlogic.com \
--cc=carlo@caione.org \
--cc=hanjie.lin@amlogic.com \
--cc=jbrunet@baylibre.com \
--cc=jianxin.pan@amlogic.com \
--cc=khilman@baylibre.com \
--cc=linus.walleij@linaro.org \
--cc=linux-amlogic@lists.infradead.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=martin.blumenstingl@googlemail.com \
--cc=narmstrong@baylibre.com \
--cc=robh+dt@kernel.org \
--cc=xingyu.chen@amlogic.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.