From: Samuel Holland <samuel@sholland.org> To: Heiko Stuebner <heiko@sntech.de>, Chen-Yu Tsai <wens@csie.org>, Jernej Skrabec <jernej.skrabec@gmail.com>, Linus Walleij <linus.walleij@linaro.org>, linux-arm-kernel@lists.infradead.org Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Maxime Ripard <mripard@kernel.org>, Ondrej Jirman <x@xff.cz>, Rob Herring <robh+dt@kernel.org>, devicetree@vger.kernel.org, linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org, linux-sunxi@lists.linux.dev Subject: Re: [PATCH 6/6] pinctrl: sunxi: Add driver for Allwinner D1/D1s Date: Fri, 1 Jul 2022 10:16:07 -0500 [thread overview] Message-ID: <e6d2d4e8-b385-11d6-4d41-454542735269@sholland.org> (raw) In-Reply-To: <17090319.JCcGWNJJiE@phil> Hi Heiko, On 7/1/22 8:13 AM, Heiko Stuebner wrote: > Am Sonntag, 26. Juni 2022, 04:11:47 CEST schrieb Samuel Holland: >> These SoCs contain a pinctrl with a new register layout. Use the variant >> parameter to set the right register offsets. This pinctrl also increases >> the number of functions per pin from 8 to 16, taking advantage of all 4 >> bits in the mux config field (so far, only functions 0-8 and 14-15 are >> used). This increases the maximum possible number of functions. >> >> D1s is a low pin count version of the D1 SoC, with some pins omitted. >> The remaining pins have the same function assignments as D1. >> >> Signed-off-by: Samuel Holland <samuel@sholland.org> > > On a D1-Nezha > Tested-by: Heiko Stuebner <heiko@sntech.de> > > Reviewed-by: Heiko Stuebner <heiko@sntech.de> > > with one remark below Thanks for reviewing the series. >> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c >> index ec7daaa5666b..350044d4c1b5 100644 >> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c >> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c >> @@ -1297,11 +1297,11 @@ static int sunxi_pinctrl_build_state(struct platform_device *pdev) >> >> /* >> * Find an upper bound for the maximum number of functions: in >> - * the worst case we have gpio_in, gpio_out, irq and up to four >> + * the worst case we have gpio_in, gpio_out, irq and up to seven >> * special functions per pin, plus one entry for the sentinel. >> * We'll reallocate that later anyway. >> */ >> - pctl->functions = kcalloc(4 * pctl->ngroups + 4, >> + pctl->functions = kcalloc(7 * pctl->ngroups + 4, >> sizeof(*pctl->functions), >> GFP_KERNEL); >> if (!pctl->functions) >> @@ -1494,9 +1494,15 @@ int sunxi_pinctrl_init_with_variant(struct platform_device *pdev, >> pctl->dev = &pdev->dev; >> pctl->desc = desc; >> pctl->variant = variant; >> - pctl->bank_mem_size = BANK_MEM_SIZE; >> - pctl->pull_regs_offset = PULL_REGS_OFFSET; >> - pctl->dlevel_field_width = DLEVEL_FIELD_WIDTH; >> + if (pctl->variant >= PINCTRL_SUN20I_D1) { >> + pctl->bank_mem_size = D1_BANK_MEM_SIZE; >> + pctl->pull_regs_offset = D1_PULL_REGS_OFFSET; >> + pctl->dlevel_field_width = D1_DLEVEL_FIELD_WIDTH; >> + } else { >> + pctl->bank_mem_size = BANK_MEM_SIZE; >> + pctl->pull_regs_offset = PULL_REGS_OFFSET; >> + pctl->dlevel_field_width = DLEVEL_FIELD_WIDTH; >> + } > > this is likely ok for _one_ variant (so for now this should be ok) but > will get ugly when there are more of them. > > So in the long term it might make sense to pass these values in from > the soc-specific driver maybe? Yes, in the long term. It took 10 years before the first layout change, so I do not expect another layout change any time soon. For now, I did not want to deal with the overhead of passing in the offsets individually, nor coming up with a name for each layout to look up the offsets from a table. (The BSP calls the variants "SUNXI_PCTL_HW_TYPE_0" and "SUNXI_PCTL_HW_TYPE_1", which is... not descriptive.) Regards, Samuel
WARNING: multiple messages have this Message-ID (diff)
From: Samuel Holland <samuel@sholland.org> To: Heiko Stuebner <heiko@sntech.de>, Chen-Yu Tsai <wens@csie.org>, Jernej Skrabec <jernej.skrabec@gmail.com>, Linus Walleij <linus.walleij@linaro.org>, linux-arm-kernel@lists.infradead.org Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Maxime Ripard <mripard@kernel.org>, Ondrej Jirman <x@xff.cz>, Rob Herring <robh+dt@kernel.org>, devicetree@vger.kernel.org, linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org, linux-sunxi@lists.linux.dev Subject: Re: [PATCH 6/6] pinctrl: sunxi: Add driver for Allwinner D1/D1s Date: Fri, 1 Jul 2022 10:16:07 -0500 [thread overview] Message-ID: <e6d2d4e8-b385-11d6-4d41-454542735269@sholland.org> (raw) In-Reply-To: <17090319.JCcGWNJJiE@phil> Hi Heiko, On 7/1/22 8:13 AM, Heiko Stuebner wrote: > Am Sonntag, 26. Juni 2022, 04:11:47 CEST schrieb Samuel Holland: >> These SoCs contain a pinctrl with a new register layout. Use the variant >> parameter to set the right register offsets. This pinctrl also increases >> the number of functions per pin from 8 to 16, taking advantage of all 4 >> bits in the mux config field (so far, only functions 0-8 and 14-15 are >> used). This increases the maximum possible number of functions. >> >> D1s is a low pin count version of the D1 SoC, with some pins omitted. >> The remaining pins have the same function assignments as D1. >> >> Signed-off-by: Samuel Holland <samuel@sholland.org> > > On a D1-Nezha > Tested-by: Heiko Stuebner <heiko@sntech.de> > > Reviewed-by: Heiko Stuebner <heiko@sntech.de> > > with one remark below Thanks for reviewing the series. >> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c >> index ec7daaa5666b..350044d4c1b5 100644 >> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c >> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c >> @@ -1297,11 +1297,11 @@ static int sunxi_pinctrl_build_state(struct platform_device *pdev) >> >> /* >> * Find an upper bound for the maximum number of functions: in >> - * the worst case we have gpio_in, gpio_out, irq and up to four >> + * the worst case we have gpio_in, gpio_out, irq and up to seven >> * special functions per pin, plus one entry for the sentinel. >> * We'll reallocate that later anyway. >> */ >> - pctl->functions = kcalloc(4 * pctl->ngroups + 4, >> + pctl->functions = kcalloc(7 * pctl->ngroups + 4, >> sizeof(*pctl->functions), >> GFP_KERNEL); >> if (!pctl->functions) >> @@ -1494,9 +1494,15 @@ int sunxi_pinctrl_init_with_variant(struct platform_device *pdev, >> pctl->dev = &pdev->dev; >> pctl->desc = desc; >> pctl->variant = variant; >> - pctl->bank_mem_size = BANK_MEM_SIZE; >> - pctl->pull_regs_offset = PULL_REGS_OFFSET; >> - pctl->dlevel_field_width = DLEVEL_FIELD_WIDTH; >> + if (pctl->variant >= PINCTRL_SUN20I_D1) { >> + pctl->bank_mem_size = D1_BANK_MEM_SIZE; >> + pctl->pull_regs_offset = D1_PULL_REGS_OFFSET; >> + pctl->dlevel_field_width = D1_DLEVEL_FIELD_WIDTH; >> + } else { >> + pctl->bank_mem_size = BANK_MEM_SIZE; >> + pctl->pull_regs_offset = PULL_REGS_OFFSET; >> + pctl->dlevel_field_width = DLEVEL_FIELD_WIDTH; >> + } > > this is likely ok for _one_ variant (so for now this should be ok) but > will get ugly when there are more of them. > > So in the long term it might make sense to pass these values in from > the soc-specific driver maybe? Yes, in the long term. It took 10 years before the first layout change, so I do not expect another layout change any time soon. For now, I did not want to deal with the overhead of passing in the offsets individually, nor coming up with a name for each layout to look up the offsets from a table. (The BSP calls the variants "SUNXI_PCTL_HW_TYPE_0" and "SUNXI_PCTL_HW_TYPE_1", which is... not descriptive.) Regards, Samuel _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-07-01 15:16 UTC|newest] Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-06-26 2:11 [PATCH 0/6] pinctrl: sunxi: Allwinner D1/D1s support Samuel Holland 2022-06-26 2:11 ` Samuel Holland 2022-06-26 2:11 ` [PATCH 1/6] dt-bindings: pinctrl: Add compatibles for Allwinner D1/D1s Samuel Holland 2022-06-26 2:11 ` Samuel Holland 2022-06-26 10:37 ` Krzysztof Kozlowski 2022-06-26 10:37 ` Krzysztof Kozlowski 2022-07-01 13:02 ` Heiko Stuebner 2022-07-01 13:02 ` Heiko Stuebner 2022-07-11 8:58 ` Linus Walleij 2022-07-11 8:58 ` Linus Walleij 2022-07-12 10:14 ` Samuel Holland 2022-07-12 10:14 ` Samuel Holland 2022-07-13 2:56 ` Samuel Holland 2022-07-13 2:56 ` Samuel Holland 2022-06-26 2:11 ` [PATCH 2/6] pinctrl: sunxi: Add I/O bias setting for H6 R-PIO Samuel Holland 2022-06-26 2:11 ` Samuel Holland 2022-06-27 20:34 ` Jernej Škrabec 2022-06-27 20:34 ` Jernej Škrabec 2022-06-28 3:18 ` Samuel Holland 2022-06-28 3:18 ` Samuel Holland 2022-07-01 13:03 ` Heiko Stuebner 2022-07-01 13:03 ` Heiko Stuebner 2022-06-26 2:11 ` [PATCH 3/6] pinctrl: sunxi: Support the 2.5V I/O bias mode Samuel Holland 2022-06-26 2:11 ` Samuel Holland 2022-06-27 20:43 ` Jernej Škrabec 2022-06-27 20:43 ` Jernej Škrabec 2022-06-28 3:29 ` Samuel Holland 2022-06-28 3:29 ` Samuel Holland 2022-07-02 19:48 ` Jernej Škrabec 2022-07-02 19:48 ` Jernej Škrabec 2022-07-01 13:04 ` Heiko Stuebner 2022-07-01 13:04 ` Heiko Stuebner 2022-06-26 2:11 ` [PATCH 4/6] pinctrl: sunxi: Refactor register/offset calculation Samuel Holland 2022-06-26 2:11 ` Samuel Holland 2022-07-01 13:07 ` Heiko Stuebner 2022-07-01 13:07 ` Heiko Stuebner 2022-07-02 20:29 ` Jernej Škrabec 2022-07-02 20:29 ` Jernej Škrabec 2022-06-26 2:11 ` [PATCH 5/6] pinctrl: sunxi: Make some layout parameters dynamic Samuel Holland 2022-06-26 2:11 ` Samuel Holland 2022-07-01 13:09 ` Heiko Stuebner 2022-07-01 13:09 ` Heiko Stuebner 2022-07-02 20:33 ` Jernej Škrabec 2022-07-02 20:33 ` Jernej Škrabec 2022-06-26 2:11 ` [PATCH 6/6] pinctrl: sunxi: Add driver for Allwinner D1/D1s Samuel Holland 2022-06-26 2:11 ` Samuel Holland 2022-07-01 13:13 ` Heiko Stuebner 2022-07-01 13:13 ` Heiko Stuebner 2022-07-01 15:16 ` Samuel Holland [this message] 2022-07-01 15:16 ` Samuel Holland 2022-07-02 14:47 ` Andre Przywara 2022-07-02 14:47 ` Andre Przywara 2022-07-02 15:43 ` Samuel Holland 2022-07-02 15:43 ` Samuel Holland 2022-07-04 9:42 ` Andre Przywara 2022-07-04 9:42 ` Andre Przywara
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=e6d2d4e8-b385-11d6-4d41-454542735269@sholland.org \ --to=samuel@sholland.org \ --cc=devicetree@vger.kernel.org \ --cc=heiko@sntech.de \ --cc=jernej.skrabec@gmail.com \ --cc=krzysztof.kozlowski+dt@linaro.org \ --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-sunxi@lists.linux.dev \ --cc=mripard@kernel.org \ --cc=robh+dt@kernel.org \ --cc=wens@csie.org \ --cc=x@xff.cz \ /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: linkBe 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.