[PATCH] pinctrl: sunxi: add SPI1 pinmuxes for f1c100s

[PATCH] pinctrl: sunxi: add SPI1 pinmuxes for f1c100s

[Iota Hydrae]

SPI1 is also usable on port A.

But there's nothing about SPI1 pinmux there for now.

This should be set when you are using SPI1.

According to the manual, the mux value would be 6.

Signed-off-by: Iota Hydrae <writeforever@foxmail.com>
---
 drivers/pinctrl/sunxi/pinctrl-sunxi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
index e5102180902..c448e40ab07 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
@@ -240,6 +240,7 @@ static const struct sunxi_pinctrl_function suniv_f1c100s_pinctrl_functions[] = {
 	{ "mmc0",	2 },	/* PF0-PF5 */
 	{ "mmc1",	3 },	/* PC0-PC2 */
 	{ "spi0",	2 },	/* PC0-PC3 */
+	{ "spi1",	6 },	/* PA0-PA3 */
 #if IS_ENABLED(CONFIG_UART0_PORT_F)
 	{ "uart0",	3 },	/* PF2-PF4 */
 #else
-- 
2.40.0

Re: [PATCH] pinctrl: sunxi: add SPI1 pinmuxes for f1c100s

[Andre Przywara]

On Wed, 23 Aug 2023 14:47:16 +0800
Iota Hydrae <writeforever@foxmail.com> wrote:

Hi,

> SPI1 is also usable on port A.
> 
> But there's nothing about SPI1 pinmux there for now.
> 
> This should be set when you are using SPI1.
> 
> According to the manual, the mux value would be 6.

So technically that looks alright, but what do you need that for? We
try to not add pinmux values to that table unless they are used
somewhere, especially if there are potential conflicting values: SPI1
on PE is on pinmux 4.
So do you have a board that needs that? Any chance you could upstream
the DT (to the kernel list!) and defconfig for that board? That would
it make it a no-brainer to merge this patch.

Cheers,
Andre

> Signed-off-by: Iota Hydrae <writeforever@foxmail.com>
> ---
>  drivers/pinctrl/sunxi/pinctrl-sunxi.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> index e5102180902..c448e40ab07 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> @@ -240,6 +240,7 @@ static const struct sunxi_pinctrl_function suniv_f1c100s_pinctrl_functions[] = {
>  	{ "mmc0",	2 },	/* PF0-PF5 */
>  	{ "mmc1",	3 },	/* PC0-PC2 */
>  	{ "spi0",	2 },	/* PC0-PC3 */
> +	{ "spi1",	6 },	/* PA0-PA3 */
>  #if IS_ENABLED(CONFIG_UART0_PORT_F)
>  	{ "uart0",	3 },	/* PF2-PF4 */
>  #else

Re: [PATCH] pinctrl: sunxi: add SPI1 pinmuxes for f1c100s

[Andre Przywara]

Hi Iota,

please keep the list and other people on CC: when replying, so that it 
stays transparent.

On 27/08/2023 23:10, Iota Hydrae wrote:
> Sorry for I didn't realized the confilcting problem.
> 
> I was developing a video driver for a small oled display (ssd1327)
> using SPI1 and It's not necessary to use it actually (board specific).
> 
> I do have a board needs this, but still got work to do and we don't
> have an defconfig or dt for that now.

I see, that's fine.

> Maybe we could add a config like "CONFIG_UART0_PORT_F" to specify
> if SPI1 on Port A? Anyway, thanks for your replay.

Nah, we don't want any more of those hacks.
If you eventually upstream your defconfig, we can take this patch, and 
any other user will have to face the problem then.
We have ideas how to fix the problem (for instance adding a port number 
to each line), but we don't want to prematurely add something without it 
really being needed.
So if you have non-mainline patches in your branch anyway, just keep 
this patch there as well, and then send all of them once you are ready.
The conflict is theoretical at this point, and we will work out a proper 
solution once this actually becomes necessary.

Cheers,
Andre

> 
> Cheers,
> Iota
> 
> On Thu, Aug 24, 2023 at 02:56:21PM +0100, Andre Przywara wrote:
>> On Wed, 23 Aug 2023 14:47:16 +0800
>> Iota Hydrae <writeforever@foxmail.com> wrote:
>>
>> Hi,
>>
>>> SPI1 is also usable on port A.
>>>
>>> But there's nothing about SPI1 pinmux there for now.
>>>
>>> This should be set when you are using SPI1.
>>>
>>> According to the manual, the mux value would be 6.
>>
>> So technically that looks alright, but what do you need that for? We
>> try to not add pinmux values to that table unless they are used
>> somewhere, especially if there are potential conflicting values: SPI1
>> on PE is on pinmux 4.
>> So do you have a board that needs that? Any chance you could upstream
>> the DT (to the kernel list!) and defconfig for that board? That would
>> it make it a no-brainer to merge this patch.
>>
>> Cheers,
>> Andre
>>
>>> Signed-off-by: Iota Hydrae <writeforever@foxmail.com>
>>> ---
>>>   drivers/pinctrl/sunxi/pinctrl-sunxi.c | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>>> index e5102180902..c448e40ab07 100644
>>> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>>> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>>> @@ -240,6 +240,7 @@ static const struct sunxi_pinctrl_function suniv_f1c100s_pinctrl_functions[] = {
>>>   	{ "mmc0",	2 },	/* PF0-PF5 */
>>>   	{ "mmc1",	3 },	/* PC0-PC2 */
>>>   	{ "spi0",	2 },	/* PC0-PC3 */
>>> +	{ "spi1",	6 },	/* PA0-PA3 */
>>>   #if IS_ENABLED(CONFIG_UART0_PORT_F)
>>>   	{ "uart0",	3 },	/* PF2-PF4 */
>>>   #else
>>
>