From: Parthiban Nallathambi <pn@denx.de>
To: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Cc: Saravanan Sekar <sravanhome@gmail.com>,
pn@denx.de, linus.walleij@linaro.org, robh+dt@kernel.org,
mark.rutland@arm.com, afaerber@suse.de, catalin.marinas@arm.com,
will.deacon@arm.com, linux-gpio@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, mp-cs@actions-semi.com,
jeff.chen@actions-semi.com, thomas.liau@actions-semi.com,
linux@cubietech.com
Subject: Re: [PATCH v5 1/5] pinctrl: actions: define constructor generic to Actions Semi SoC's
Date: Mon, 12 Nov 2018 17:27:12 +0100 [thread overview]
Message-ID: <4a54e87e-cf8d-7037-7b7a-4e30e27338da@denx.de> (raw)
In-Reply-To: <72d4911a-588f-9bf3-4fad-a0b34692ecd4@denx.de>
Hi Mani,
On 11/6/18 5:43 PM, Parthiban Nallathambi wrote:
> ping on this patch series!
>
> On 9/4/18 8:41 PM, Saravanan Sekar wrote:
>>
>>
>> On 08/29/18 17:20, Manivannan Sadhasivam wrote:
>>> Hi Saravanan,
>>>
>>> On Wed, Aug 29, 2018 at 10:24:09AM +0200, Saravanan Sekar wrote:
>>>> Move generic defines common to the Owl family out of S900 driver.
>>>>
>>>> Signed-off-by: Parthiban Nallathambi <pn@denx.de>
>>>> Signed-off-by: Saravanan Sekar <sravanhome@gmail.com>
>>>> ---
>>>> drivers/pinctrl/actions/pinctrl-owl.h | 131
>>>> +++++++++++++++++++++++++++++++
>>>> drivers/pinctrl/actions/pinctrl-s900.c | 139
>>>> ++-------------------------------
>>>> 2 files changed, 137 insertions(+), 133 deletions(-)
>>>>
>>>> diff --git a/drivers/pinctrl/actions/pinctrl-owl.h
>>>> b/drivers/pinctrl/actions/pinctrl-owl.h
>>>> index a724d1d406d4..31cc33d7c4a5 100644
>>>> --- a/drivers/pinctrl/actions/pinctrl-owl.h
>>>> +++ b/drivers/pinctrl/actions/pinctrl-owl.h
>>>> @@ -15,6 +15,136 @@
>>>> #define OWL_PINCONF_SLEW_SLOW 0
>>>> #define OWL_PINCONF_SLEW_FAST 1
>>>> +#define MUX_PG(group_name, reg, shift, width) \
>>>> + { \
>>>> + .name = #group_name, \
>>>> + .pads = group_name##_pads, \
>>>> + .npads = ARRAY_SIZE(group_name##_pads), \
>>>> + .funcs = group_name##_funcs, \
>>>> + .nfuncs = ARRAY_SIZE(group_name##_funcs), \
>>>> + .mfpctl_reg = MFCTL##reg, \
>>>> + .mfpctl_shift = shift, \
>>>> + .mfpctl_width = width, \
>>>> + .drv_reg = -1, \
>>>> + .drv_shift = -1, \
>>>> + .drv_width = -1, \
>>>> + .sr_reg = -1, \
>>>> + .sr_shift = -1, \
>>>> + .sr_width = -1, \
>>>> + }
>>>> +
>>>> +#define DRV_PG(group_name, reg, shift, width) \
>>>> + { \
>>>> + .name = #group_name, \
>>>> + .pads = group_name##_pads, \
>>>> + .npads = ARRAY_SIZE(group_name##_pads), \
>>>> + .mfpctl_reg = -1, \
>>>> + .mfpctl_shift = -1, \
>>>> + .mfpctl_width = -1, \
>>>> + .drv_reg = PAD_DRV##reg, \
>>>> + .drv_shift = shift, \
>>>> + .drv_width = width, \
>>>> + .sr_reg = -1, \
>>>> + .sr_shift = -1, \
>>>> + .sr_width = -1, \
>>>> + }
>>>> +
>>>> +#define SR_PG(group_name, reg, shift, width) \
>>>> + { \
>>>> + .name = #group_name, \
>>>> + .pads = group_name##_pads, \
>>>> + .npads = ARRAY_SIZE(group_name##_pads), \
>>>> + .mfpctl_reg = -1, \
>>>> + .mfpctl_shift = -1, \
>>>> + .mfpctl_width = -1, \
>>>> + .drv_reg = -1, \
>>>> + .drv_shift = -1, \
>>>> + .drv_width = -1, \
>>>> + .sr_reg = PAD_SR##reg, \
>>>> + .sr_shift = shift, \
>>>> + .sr_width = width, \
>>>> + }
>>>> +
>>>> +#define FUNCTION(fname) \
>>>> + { \
>>>> + .name = #fname, \
>>>> + .groups = fname##_groups, \
>>>> + .ngroups = ARRAY_SIZE(fname##_groups), \
>>>> + }
>>>> +
>>>> +/* PAD PULL UP/DOWN CONFIGURES */
>>>> +#define PULLCTL_CONF(pull_reg, pull_sft, pull_wdt) \
>>>> + { \
>>>> + .reg = PAD_PULLCTL##pull_reg, \
>>>> + .shift = pull_sft, \
>>>> + .width = pull_wdt, \
>>>> + }
>>>> +
>>>> +#define PAD_PULLCTL_CONF(pad_name, pull_reg, pull_sft, pull_wdt) \
>>>> + struct owl_pullctl pad_name##_pullctl_conf \
>>>> + = PULLCTL_CONF(pull_reg, pull_sft, pull_wdt)
>>>> +
>>>> +#define ST_CONF(st_reg, st_sft, st_wdt) \
>>>> + { \
>>>> + .reg = PAD_ST##st_reg, \
>>>> + .shift = st_sft, \
>>>> + .width = st_wdt, \
>>>> + }
>>>> +
>>>> +#define PAD_ST_CONF(pad_name, st_reg, st_sft, st_wdt) \
>>>> + struct owl_st pad_name##_st_conf \
>>>> + = ST_CONF(st_reg, st_sft, st_wdt)
>>>> +
>>>> +#define PAD_INFO(name) \
>>>> + { \
>>>> + .pad = name, \
>>>> + .pullctl = NULL, \
>>>> + .st = NULL, \
>>>> + }
>>>> +
>>>> +#define PAD_INFO_ST(name) \
>>>> + { \
>>>> + .pad = name, \
>>>> + .pullctl = NULL, \
>>>> + .st = &name##_st_conf, \
>>>> + }
>>>> +
>>>> +#define PAD_INFO_PULLCTL(name) \
>>>> + { \
>>>> + .pad = name, \
>>>> + .pullctl = &name##_pullctl_conf, \
>>>> + .st = NULL, \
>>>> + }
>>>> +
>>>> +#define PAD_INFO_PULLCTL_ST(name) \
>>>> + { \
>>>> + .pad = name, \
>>>> + .pullctl = &name##_pullctl_conf, \
>>>> + .st = &name##_st_conf, \
>>>> + }
>>>> +
>>>> +#define OWL_GPIO_PORT_A 0
>>>> +#define OWL_GPIO_PORT_B 1
>>>> +#define OWL_GPIO_PORT_C 2
>>>> +#define OWL_GPIO_PORT_D 3
>>>> +#define OWL_GPIO_PORT_E 4
>>>> +#define OWL_GPIO_PORT_F 5
>>>> +
>>>> +#define OWL_GPIO_PORT(port, base, count, _outen, _inen, _dat,
>>>> _intc_ctl,\
>>>> + _intc_pd, _intc_msk, _intc_type, _share) \
>>>> + [OWL_GPIO_PORT_##port] = { \
>>>> + .offset = base, \
>>>> + .pins = count, \
>>>> + .outen = _outen, \
>>>> + .inen = _inen, \
>>>> + .dat = _dat, \
>>>> + .intc_ctl = _intc_ctl, \
>>>> + .intc_pd = _intc_pd, \
>>>> + .intc_msk = _intc_msk, \
>>>> + .intc_type = _intc_type, \
>>>> + .shared_ctl_offset = _share, \
>>>
>>> Above member addition is not related to this patch. What is the purpose
>>> of this? Either move this to a separate commit if it is needed by S700
>>> or justify in this patch itself.
>>>
>>
>> This is introduced for S700 as registers are shared, it is not case in
>> S900. There is impact on S900 also, which needs another seperate patch
>> which is unlikely as we cleaning S700 related stuffs here
S700 shares the control register (INTC_GPIOCTL) for all the GPIO ports.
Hence this is introduced only for s700 to offset into the control
register for configuring (int pending/enable/clock select).
So should it be separate commit still? As mentioned Saravanan above,
this has the impact in s900 as well.
>>
>>> Btw, this patch is not applying cleanly.
Will send v6 based on current mainline master!
Thanks,
Parthi
>>>
>>>> + }
>>>> +
>>>> enum owl_pinconf_pull {
>>>> OWL_PINCONF_PULL_HIZ,
>>>> OWL_PINCONF_PULL_DOWN,
>>>> @@ -148,6 +278,7 @@ struct owl_gpio_port {
>>>> unsigned int intc_pd;
>>>> unsigned int intc_msk;
>>>> unsigned int intc_type;
>>>> + u8 shared_ctl_offset;
>>>
>>> Same as above.
>>>
>>> Regards,
>>> Mani
>>>
>>>> };
>>>> /**
>>>> diff --git a/drivers/pinctrl/actions/pinctrl-s900.c
>>>> b/drivers/pinctrl/actions/pinctrl-s900.c
>>>> index ea67b14ef93b..0597009d8369 100644
>>>> --- a/drivers/pinctrl/actions/pinctrl-s900.c
>>>> +++ b/drivers/pinctrl/actions/pinctrl-s900.c
>>>> @@ -33,13 +33,6 @@
>>>> #define PAD_SR1 (0x0274)
>>>> #define PAD_SR2 (0x0278)
>>>> -#define OWL_GPIO_PORT_A 0
>>>> -#define OWL_GPIO_PORT_B 1
>>>> -#define OWL_GPIO_PORT_C 2
>>>> -#define OWL_GPIO_PORT_D 3
>>>> -#define OWL_GPIO_PORT_E 4
>>>> -#define OWL_GPIO_PORT_F 5
>>>> -
>>>> #define _GPIOA(offset) (offset)
>>>> #define _GPIOB(offset) (32 + (offset))
>>>> #define _GPIOC(offset) (64 + (offset))
>>>> @@ -892,55 +885,6 @@ static unsigned int i2c2_sr_pads[] = {
>>>> I2C2_SCLK, I2C2_SDATA };
>>>> static unsigned int sensor0_sr_pads[] = { SENSOR0_PCLK,
>>>> SENSOR0_CKOUT };
>>>> -#define MUX_PG(group_name, reg, shift, width) \
>>>> - { \
>>>> - .name = #group_name, \
>>>> - .pads = group_name##_pads, \
>>>> - .npads = ARRAY_SIZE(group_name##_pads), \
>>>> - .funcs = group_name##_funcs, \
>>>> - .nfuncs = ARRAY_SIZE(group_name##_funcs), \
>>>> - .mfpctl_reg = MFCTL##reg, \
>>>> - .mfpctl_shift = shift, \
>>>> - .mfpctl_width = width, \
>>>> - .drv_reg = -1, \
>>>> - .drv_shift = -1, \
>>>> - .drv_width = -1, \
>>>> - .sr_reg = -1, \
>>>> - .sr_shift = -1, \
>>>> - .sr_width = -1, \
>>>> - }
>>>> -
>>>> -#define DRV_PG(group_name, reg, shift, width) \
>>>> - { \
>>>> - .name = #group_name, \
>>>> - .pads = group_name##_pads, \
>>>> - .npads = ARRAY_SIZE(group_name##_pads), \
>>>> - .mfpctl_reg = -1, \
>>>> - .mfpctl_shift = -1, \
>>>> - .mfpctl_width = -1, \
>>>> - .drv_reg = PAD_DRV##reg, \
>>>> - .drv_shift = shift, \
>>>> - .drv_width = width, \
>>>> - .sr_reg = -1, \
>>>> - .sr_shift = -1, \
>>>> - .sr_width = -1, \
>>>> - }
>>>> -
>>>> -#define SR_PG(group_name, reg, shift, width) \
>>>> - { \
>>>> - .name = #group_name, \
>>>> - .pads = group_name##_pads, \
>>>> - .npads = ARRAY_SIZE(group_name##_pads), \
>>>> - .mfpctl_reg = -1, \
>>>> - .mfpctl_shift = -1, \
>>>> - .mfpctl_width = -1, \
>>>> - .drv_reg = -1, \
>>>> - .drv_shift = -1, \
>>>> - .drv_width = -1, \
>>>> - .sr_reg = PAD_SR##reg, \
>>>> - .sr_shift = shift, \
>>>> - .sr_width = width, \
>>>> - }
>>>> /* Pinctrl groups */
>>>> static const struct owl_pingroup s900_groups[] = {
>>>> @@ -1442,13 +1386,6 @@ static const char * const sirq2_groups[] = {
>>>> "sirq2_dummy",
>>>> };
>>>> -#define FUNCTION(fname) \
>>>> - { \
>>>> - .name = #fname, \
>>>> - .groups = fname##_groups, \
>>>> - .ngroups = ARRAY_SIZE(fname##_groups), \
>>>> - }
>>>> -
>>>> static const struct owl_pinmux_func s900_functions[] = {
>>>> [S900_MUX_ERAM] = FUNCTION(eram),
>>>> [S900_MUX_ETH_RMII] = FUNCTION(eth_rmii),
>>>> @@ -1500,28 +1437,6 @@ static const struct owl_pinmux_func
>>>> s900_functions[] = {
>>>> [S900_MUX_SIRQ1] = FUNCTION(sirq1),
>>>> [S900_MUX_SIRQ2] = FUNCTION(sirq2)
>>>> };
>>>> -/* PAD PULL UP/DOWN CONFIGURES */
>>>> -#define PULLCTL_CONF(pull_reg, pull_sft, pull_wdt) \
>>>> - { \
>>>> - .reg = PAD_PULLCTL##pull_reg, \
>>>> - .shift = pull_sft, \
>>>> - .width = pull_wdt, \
>>>> - }
>>>> -
>>>> -#define PAD_PULLCTL_CONF(pad_name, pull_reg, pull_sft, pull_wdt) \
>>>> - struct owl_pullctl pad_name##_pullctl_conf \
>>>> - = PULLCTL_CONF(pull_reg, pull_sft, pull_wdt)
>>>> -
>>>> -#define ST_CONF(st_reg, st_sft, st_wdt) \
>>>> - { \
>>>> - .reg = PAD_ST##st_reg, \
>>>> - .shift = st_sft, \
>>>> - .width = st_wdt, \
>>>> - }
>>>> -
>>>> -#define PAD_ST_CONF(pad_name, st_reg, st_sft, st_wdt) \
>>>> - struct owl_st pad_name##_st_conf \
>>>> - = ST_CONF(st_reg, st_sft, st_wdt)
>>>> /* PAD_PULLCTL0 */
>>>> static PAD_PULLCTL_CONF(ETH_RXER, 0, 18, 2);
>>>> @@ -1639,34 +1554,6 @@ static PAD_ST_CONF(SPI0_SS, 1, 2, 1);
>>>> static PAD_ST_CONF(I2S_BCLK0, 1, 1, 1);
>>>> static PAD_ST_CONF(I2S_MCLK0, 1, 0, 1);
>>>> -#define PAD_INFO(name) \
>>>> - { \
>>>> - .pad = name, \
>>>> - .pullctl = NULL, \
>>>> - .st = NULL, \
>>>> - }
>>>> -
>>>> -#define PAD_INFO_ST(name) \
>>>> - { \
>>>> - .pad = name, \
>>>> - .pullctl = NULL, \
>>>> - .st = &name##_st_conf, \
>>>> - }
>>>> -
>>>> -#define PAD_INFO_PULLCTL(name) \
>>>> - { \
>>>> - .pad = name, \
>>>> - .pullctl = &name##_pullctl_conf, \
>>>> - .st = NULL, \
>>>> - }
>>>> -
>>>> -#define PAD_INFO_PULLCTL_ST(name) \
>>>> - { \
>>>> - .pad = name, \
>>>> - .pullctl = &name##_pullctl_conf, \
>>>> - .st = &name##_st_conf, \
>>>> - }
>>>> -
>>>> /* Pad info table */
>>>> static struct owl_padinfo s900_padinfo[NUM_PADS] = {
>>>> [ETH_TXD0] = PAD_INFO_ST(ETH_TXD0),
>>>> @@ -1821,27 +1708,13 @@ static struct owl_padinfo
>>>> s900_padinfo[NUM_PADS] = {
>>>> [SGPIO3] = PAD_INFO_PULLCTL_ST(SGPIO3)
>>>> };
>>>> -#define OWL_GPIO_PORT(port, base, count, _outen, _inen, _dat, \
>>>> - _intc_ctl, _intc_pd, _intc_msk, _intc_type) \
>>>> - [OWL_GPIO_PORT_##port] = { \
>>>> - .offset = base, \
>>>> - .pins = count, \
>>>> - .outen = _outen, \
>>>> - .inen = _inen, \
>>>> - .dat = _dat, \
>>>> - .intc_ctl = _intc_ctl, \
>>>> - .intc_pd = _intc_pd, \
>>>> - .intc_msk = _intc_msk, \
>>>> - .intc_type = _intc_type, \
>>>> - }
>>>> -
>>>> static const struct owl_gpio_port s900_gpio_ports[] = {
>>>> - OWL_GPIO_PORT(A, 0x0000, 32, 0x0, 0x4, 0x8, 0x204, 0x208,
>>>> 0x20C, 0x240),
>>>> - OWL_GPIO_PORT(B, 0x000C, 32, 0x0, 0x4, 0x8, 0x534, 0x204,
>>>> 0x208, 0x23C),
>>>> - OWL_GPIO_PORT(C, 0x0018, 12, 0x0, 0x4, 0x8, 0x52C, 0x200,
>>>> 0x204, 0x238),
>>>> - OWL_GPIO_PORT(D, 0x0024, 30, 0x0, 0x4, 0x8, 0x524, 0x1FC,
>>>> 0x200, 0x234),
>>>> - OWL_GPIO_PORT(E, 0x0030, 32, 0x0, 0x4, 0x8, 0x51C, 0x1F8,
>>>> 0x1FC, 0x230),
>>>> - OWL_GPIO_PORT(F, 0x00F0, 8, 0x0, 0x4, 0x8, 0x460, 0x140, 0x144,
>>>> 0x178)
>>>> + OWL_GPIO_PORT(A, 0x0000, 32, 0x0, 0x4, 0x8, 0x204, 0x208,
>>>> 0x20C, 0x240, 0),
>>>> + OWL_GPIO_PORT(B, 0x000C, 32, 0x0, 0x4, 0x8, 0x534, 0x204,
>>>> 0x208, 0x23C, 0),
>>>> + OWL_GPIO_PORT(C, 0x0018, 12, 0x0, 0x4, 0x8, 0x52C, 0x200,
>>>> 0x204, 0x238, 0),
>>>> + OWL_GPIO_PORT(D, 0x0024, 30, 0x0, 0x4, 0x8, 0x524, 0x1FC,
>>>> 0x200, 0x234, 0),
>>>> + OWL_GPIO_PORT(E, 0x0030, 32, 0x0, 0x4, 0x8, 0x51C, 0x1F8,
>>>> 0x1FC, 0x230, 0),
>>>> + OWL_GPIO_PORT(F, 0x00F0, 8, 0x0, 0x4, 0x8, 0x460, 0x140, 0x144,
>>>> 0x178, 0)
>>>> };
>>>> static struct owl_pinctrl_soc_data s900_pinctrl_data = {
>>>> --
>>>> 2.14.4
>>>>
>>
>
--
Thanks,
Parthiban N
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-22 Fax: (+49)-8142-66989-80 Email: pn@denx.de
next prev parent reply other threads:[~2018-11-12 16:27 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-29 8:24 [PATCH v5 0/5] Add Actions Semi S700 pinctrl support Saravanan Sekar
2018-08-29 8:24 ` [PATCH v5 1/5] pinctrl: actions: define constructor generic to Actions Semi SoC's Saravanan Sekar
2018-08-29 15:20 ` Manivannan Sadhasivam
2018-09-04 18:41 ` Saravanan Sekar
2018-11-06 16:43 ` Parthiban Nallathambi
2018-11-09 9:43 ` Linus Walleij
2018-11-12 16:27 ` Parthiban Nallathambi [this message]
2018-08-29 8:24 ` [PATCH v5 2/5] pinctrl: actions: define pad control configurtion to SoC specific Saravanan Sekar
2018-08-29 15:32 ` Manivannan Sadhasivam
2018-11-12 16:29 ` Parthiban Nallathambi
2018-08-29 8:24 ` [PATCH v5 3/5] dt-bindings: pinctrl: Add bindings for Actions Semi S700 SoC Saravanan Sekar
2018-08-29 15:50 ` Manivannan Sadhasivam
2018-09-04 18:57 ` Saravanan Sekar
2018-11-06 16:44 ` Parthiban Nallathambi
2018-11-12 16:32 ` Parthiban Nallathambi
2018-08-29 8:24 ` [PATCH v5 4/5] pinctrl: actions: Add Actions Semi S700 pinctrl driver Saravanan Sekar
2018-08-29 16:34 ` Manivannan Sadhasivam
2018-09-04 19:28 ` Saravanan Sekar
2018-11-06 16:44 ` Parthiban Nallathambi
2018-11-06 17:25 ` Manivannan Sadhasivam
2018-11-06 18:02 ` Parthiban Nallathambi
2018-08-29 8:24 ` [PATCH v5 5/5] arm64: dts: actions: Add pinctrl node for Actions Semi S700 Saravanan Sekar
2018-08-30 17:05 ` kbuild test robot
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=4a54e87e-cf8d-7037-7b7a-4e30e27338da@denx.de \
--to=pn@denx.de \
--cc=afaerber@suse.de \
--cc=catalin.marinas@arm.com \
--cc=devicetree@vger.kernel.org \
--cc=jeff.chen@actions-semi.com \
--cc=linus.walleij@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@cubietech.com \
--cc=manivannan.sadhasivam@linaro.org \
--cc=mark.rutland@arm.com \
--cc=mp-cs@actions-semi.com \
--cc=robh+dt@kernel.org \
--cc=sravanhome@gmail.com \
--cc=thomas.liau@actions-semi.com \
--cc=will.deacon@arm.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).