Mailing List Archive: pinctrl: zynqmp: Valid pin muxings cannot be configured

pinctrl: zynqmp: Valid pin muxings cannot be configured

Apr 23, 2024, 4:04 PM

Post #1 of 4 (5 views)

Hi Michal,

I was looking to upstream one of our ZynqMP boards, and I ran into an
issue with the pinmuxing. We use almost all of the I/Os, so everything
is tightly packed into the MIO. For example, we have the QSPI on MIO0 to
MIO5, and MIO6 to MIO11 are used for SPI1. However, I cannot select this
configuration using the pinmux driver. I am using the following
configuration:

pinctrl_qspi_default: qspi-default {
mux {
groups = "qspi0_0_grp";
function = "qspi0";
};

mux-cs {
groups = "qspi_ss_0_grp";
function = "qspi_ss";
};
};

pinctrl_spi1_default: spi1-default {
mux {
groups = "spi1_0_grp";
function = "spi1";
};

mux-cs {
groups = "spi1_ss_0_grp", "spi1_ss_1_grp";
function = "spi1_ss";
};
};

But I get the following errors on boot:

[ 4.261739] zynqmp-pinctrl firmware:zynqmp-firmware:pinctrl: pin MIO8 already requested by ff050000.spi; cannot claim for ff0f0000.spi
[ 4.274506] zynqmp-pinctrl firmware:zynqmp-firmware:pinctrl: error -EINVAL: pin-8 (ff0f0000.spi)
[ 4.283789] zynqmp-pinctrl firmware:zynqmp-firmware:pinctrl: error -EINVAL: could not request pin 8 (MIO8) from group qspi0_0_grp on device zynqmp_pinctrl

This is because the qspi0_0_grp and spi1_0_grp groups overlap:

group: qspi0_0_grp
pin 0 (MIO0)
pin 1 (MIO1)
pin 2 (MIO2)
pin 3 (MIO3)
pin 4 (MIO4)
pin 8 (MIO8)
pin 9 (MIO9)
pin 10 (MIO10)
pin 11 (MIO11)
pin 12 (MIO12)

group: qspi_ss_0_grp
pin 5 (MIO5)
pin 7 (MIO7)

group: qspi_fbclk_0_grp
pin 6 (MIO6)

group: spi1_0_grp
pin 6 (MIO6)
pin 10 (MIO10)
pin 11 (MIO11)

group: spi1_ss_0_grp
pin 9 (MIO9)

group: spi1_ss_1_grp
pin 8 (MIO8)

group: spi1_ss_2_grp
pin 7 (MIO7)

However, we are not using the "upper" pins of the QSPI device.
Therefore, these pins should not be included in the qspi0_0_grp. This
stems from the driver placing all possible pins into a function's group,
even though each pin can be muxed individially and it is not necessary
to mux all pins for full functionality.

I think it would be better to have a single group for each pin:

pinctrl_qspi_default: qspi-default {
mux {
groups = "mio0", "mio1", "mio2", "mio3", "mio4";
function = "qspi0";
};

mux-cs {
groups = "mio5";
function = "qspi_ss";
};
};

pinctrl_spi1_default: spi1-default {
mux {
groups = "mio6", "mio10", "mio11";
function = "spi1";
};

mux-cs {
groups = "mio8", "mio9";
function = "spi1_ss";
};
};

This allows the full functionality of this chip to be configured. Does
that sound good? I can send a patch to this effect if you agree.

--Sean

Re: pinctrl: zynqmp: Valid pin muxings cannot be configured [ In reply to ]

michal.simek at amd

Apr 23, 2024, 11:22 PM

Post #2 of 4 (3 views)

Permalink

Hi Sean,

On 4/24/24 01:04, Sean Anderson wrote:
> Hi Michal,
>
> I was looking to upstream one of our ZynqMP boards, and I ran into an
> issue with the pinmuxing. We use almost all of the I/Os, so everything
> is tightly packed into the MIO. For example, we have the QSPI on MIO0 to
> MIO5, and MIO6 to MIO11 are used for SPI1. However, I cannot select this
> configuration using the pinmux driver. I am using the following
> configuration:
>
> pinctrl_qspi_default: qspi-default {
> mux {
> groups = "qspi0_0_grp";
> function = "qspi0";
> };
>
> mux-cs {
> groups = "qspi_ss_0_grp";
> function = "qspi_ss";
> };
> };
>
> pinctrl_spi1_default: spi1-default {
> mux {
> groups = "spi1_0_grp";
> function = "spi1";
> };
>
> mux-cs {
> groups = "spi1_ss_0_grp", "spi1_ss_1_grp";
> function = "spi1_ss";
> };
> };
>
> But I get the following errors on boot:
>
> [ 4.261739] zynqmp-pinctrl firmware:zynqmp-firmware:pinctrl: pin MIO8 already requested by ff050000.spi; cannot claim for ff0f0000.spi
> [ 4.274506] zynqmp-pinctrl firmware:zynqmp-firmware:pinctrl: error -EINVAL: pin-8 (ff0f0000.spi)
> [ 4.283789] zynqmp-pinctrl firmware:zynqmp-firmware:pinctrl: error -EINVAL: could not request pin 8 (MIO8) from group qspi0_0_grp on device zynqmp_pinctrl
>
> This is because the qspi0_0_grp and spi1_0_grp groups overlap:
>
> group: qspi0_0_grp
> pin 0 (MIO0)
> pin 1 (MIO1)
> pin 2 (MIO2)
> pin 3 (MIO3)
> pin 4 (MIO4)
> pin 8 (MIO8)
> pin 9 (MIO9)
> pin 10 (MIO10)
> pin 11 (MIO11)
> pin 12 (MIO12)
>
> group: qspi_ss_0_grp
> pin 5 (MIO5)
> pin 7 (MIO7)
>
> group: qspi_fbclk_0_grp
> pin 6 (MIO6)
>
> group: spi1_0_grp
> pin 6 (MIO6)
> pin 10 (MIO10)
> pin 11 (MIO11)
>
> group: spi1_ss_0_grp
> pin 9 (MIO9)
>
> group: spi1_ss_1_grp
> pin 8 (MIO8)
>
> group: spi1_ss_2_grp
> pin 7 (MIO7)
>
> However, we are not using the "upper" pins of the QSPI device.
> Therefore, these pins should not be included in the qspi0_0_grp. This
> stems from the driver placing all possible pins into a function's group,
> even though each pin can be muxed individially and it is not necessary
> to mux all pins for full functionality.

Correct. These configurations were not consider at that time when code was
written. The same issue is there if you want to combine pins from different
groups. IIRC uart rx via MIOX and tx not from MIOX+1.

>
> I think it would be better to have a single group for each pin:
>
> pinctrl_qspi_default: qspi-default {
> mux {
> groups = "mio0", "mio1", "mio2", "mio3", "mio4";
> function = "qspi0";
> };
>
> mux-cs {
> groups = "mio5";
> function = "qspi_ss";
> };
> };
>
> pinctrl_spi1_default: spi1-default {
> mux {
> groups = "mio6", "mio10", "mio11";
> function = "spi1";
> };
>
> mux-cs {
> groups = "mio8", "mio9";
> function = "spi1_ss";
> };
> };
>
> This allows the full functionality of this chip to be configured. Does
> that sound good? I can send a patch to this effect if you agree.

The only question is if this can be done without changing TF-A code because we
are running out of space in OCM for it.

Just a generic question to your problem. It doesn't sound like a dynamic case.
You have static assignment for pins which likely won't change over lifecycle.
QSPI can be even boot device. Do you really need to describe pins via DT that it
is not enough to have them configured via psu_init directly?
Driver has been developed for i2c bus recovery via gpio which was the main
application. Right now Kria SOM is using it for carrier card pins configuration.
And Kria is pretty much only platform where this is regularly tested.

Thanks,
Michal

Re: pinctrl: zynqmp: Valid pin muxings cannot be configured [ In reply to ]

sean.anderson at linux

Apr 25, 2024, 8:22 AM

Post #3 of 4 (2 views)

Permalink

On 4/24/24 02:22, Michal Simek wrote:
> Hi Sean,
>
> On 4/24/24 01:04, Sean Anderson wrote:
>> Hi Michal,
>>
>> I was looking to upstream one of our ZynqMP boards, and I ran into an
>> issue with the pinmuxing. We use almost all of the I/Os, so everything
>> is tightly packed into the MIO. For example, we have the QSPI on MIO0 to
>> MIO5, and MIO6 to MIO11 are used for SPI1. However, I cannot select this
>> configuration using the pinmux driver. I am using the following
>> configuration:
>>
>> pinctrl_qspi_default: qspi-default {
>> mux {
>> groups = "qspi0_0_grp";
>> function = "qspi0";
>> };
>>
>> mux-cs {
>> groups = "qspi_ss_0_grp";
>> function = "qspi_ss";
>> };
>> };
>>
>> pinctrl_spi1_default: spi1-default {
>> mux {
>> groups = "spi1_0_grp";
>> function = "spi1";
>> };
>>
>> mux-cs {
>> groups = "spi1_ss_0_grp", "spi1_ss_1_grp";
>> function = "spi1_ss";
>> };
>> };
>>
>> But I get the following errors on boot:
>>
>> [ 4.261739] zynqmp-pinctrl firmware:zynqmp-firmware:pinctrl: pin MIO8 already requested by ff050000.spi; cannot claim for ff0f0000.spi
>> [ 4.274506] zynqmp-pinctrl firmware:zynqmp-firmware:pinctrl: error -EINVAL: pin-8 (ff0f0000.spi)
>> [ 4.283789] zynqmp-pinctrl firmware:zynqmp-firmware:pinctrl: error -EINVAL: could not request pin 8 (MIO8) from group qspi0_0_grp on device zynqmp_pinctrl
>>
>> This is because the qspi0_0_grp and spi1_0_grp groups overlap:
>>
>> group: qspi0_0_grp
>> pin 0 (MIO0)
>> pin 1 (MIO1)
>> pin 2 (MIO2)
>> pin 3 (MIO3)
>> pin 4 (MIO4)
>> pin 8 (MIO8)
>> pin 9 (MIO9)
>> pin 10 (MIO10)
>> pin 11 (MIO11)
>> pin 12 (MIO12)
>>
>> group: qspi_ss_0_grp
>> pin 5 (MIO5)
>> pin 7 (MIO7)
>>
>> group: qspi_fbclk_0_grp
>> pin 6 (MIO6)
>>
>> group: spi1_0_grp
>> pin 6 (MIO6)
>> pin 10 (MIO10)
>> pin 11 (MIO11)
>>
>> group: spi1_ss_0_grp
>> pin 9 (MIO9)
>>
>> group: spi1_ss_1_grp
>> pin 8 (MIO8)
>>
>> group: spi1_ss_2_grp
>> pin 7 (MIO7)
>>
>> However, we are not using the "upper" pins of the QSPI device.
>> Therefore, these pins should not be included in the qspi0_0_grp. This
>> stems from the driver placing all possible pins into a function's group,
>> even though each pin can be muxed individially and it is not necessary
>> to mux all pins for full functionality.
>
> Correct. These configurations were not consider at that time when code
> was written. The same issue is there if you want to combine pins from
> different groups. IIRC uart rx via MIOX and tx not from MIOX+1.
>
>>
>> I think it would be better to have a single group for each pin:
>>
>> pinctrl_qspi_default: qspi-default {
>> mux {
>> groups = "mio0", "mio1", "mio2", "mio3", "mio4";
>> function = "qspi0";
>> };
>>
>> mux-cs {
>> groups = "mio5";
>> function = "qspi_ss";
>> };
>> };
>>
>> pinctrl_spi1_default: spi1-default {
>> mux {
>> groups = "mio6", "mio10", "mio11";
>> function = "spi1";
>> };
>>
>> mux-cs {
>> groups = "mio8", "mio9";
>> function = "spi1_ss";
>> };
>> };
>>
>> This allows the full functionality of this chip to be configured. Does
>> that sound good? I can send a patch to this effect if you agree.
>
> The only question is if this can be done without changing TF-A code
> because we are running out of space in OCM for it.

I think this can be done just in the Linux driver. This would also be
convenient because then it will work regardless of the firmware.

> Just a generic question to your problem. It doesn't sound like a
> dynamic case. You have static assignment for pins which likely won't
> change over lifecycle. QSPI can be even boot device. Do you really
> need to describe pins via DT that it is not enough to have them
> configured via psu_init directly?
>
> Driver has been developed for i2c bus recovery via gpio which was the
> main application. Right now Kria SOM is using it for carrier card pins
> configuration. And Kria is pretty much only platform where this is
> regularly tested.

This is just following the example of the ZCU102. But you're right,
these pin configurations are static (excepting I2C).

--Sean

Re: pinctrl: zynqmp: Valid pin muxings cannot be configured [ In reply to ]

michal.simek at amd

Apr 25, 2024, 10:58 PM

Post #4 of 4 (2 views)

Permalink

On 4/25/24 17:22, Sean Anderson wrote:
> On 4/24/24 02:22, Michal Simek wrote:
>> Hi Sean,
>>
>> On 4/24/24 01:04, Sean Anderson wrote:
>>> Hi Michal,
>>>
>>> I was looking to upstream one of our ZynqMP boards, and I ran into an
>>> issue with the pinmuxing. We use almost all of the I/Os, so everything
>>> is tightly packed into the MIO. For example, we have the QSPI on MIO0 to
>>> MIO5, and MIO6 to MIO11 are used for SPI1. However, I cannot select this
>>> configuration using the pinmux driver. I am using the following
>>> configuration:
>>>
>>> pinctrl_qspi_default: qspi-default {
>>> mux {
>>> groups = "qspi0_0_grp";
>>> function = "qspi0";
>>> };
>>>
>>> mux-cs {
>>> groups = "qspi_ss_0_grp";
>>> function = "qspi_ss";
>>> };
>>> };
>>>
>>> pinctrl_spi1_default: spi1-default {
>>> mux {
>>> groups = "spi1_0_grp";
>>> function = "spi1";
>>> };
>>>
>>> mux-cs {
>>> groups = "spi1_ss_0_grp", "spi1_ss_1_grp";
>>> function = "spi1_ss";
>>> };
>>> };
>>>
>>> But I get the following errors on boot:
>>>
>>> [ 4.261739] zynqmp-pinctrl firmware:zynqmp-firmware:pinctrl: pin MIO8 already requested by ff050000.spi; cannot claim for ff0f0000.spi
>>> [ 4.274506] zynqmp-pinctrl firmware:zynqmp-firmware:pinctrl: error -EINVAL: pin-8 (ff0f0000.spi)
>>> [ 4.283789] zynqmp-pinctrl firmware:zynqmp-firmware:pinctrl: error -EINVAL: could not request pin 8 (MIO8) from group qspi0_0_grp on device zynqmp_pinctrl
>>>
>>> This is because the qspi0_0_grp and spi1_0_grp groups overlap:
>>>
>>> group: qspi0_0_grp
>>> pin 0 (MIO0)
>>> pin 1 (MIO1)
>>> pin 2 (MIO2)
>>> pin 3 (MIO3)
>>> pin 4 (MIO4)
>>> pin 8 (MIO8)
>>> pin 9 (MIO9)
>>> pin 10 (MIO10)
>>> pin 11 (MIO11)
>>> pin 12 (MIO12)
>>>
>>> group: qspi_ss_0_grp
>>> pin 5 (MIO5)
>>> pin 7 (MIO7)
>>>
>>> group: qspi_fbclk_0_grp
>>> pin 6 (MIO6)
>>>
>>> group: spi1_0_grp
>>> pin 6 (MIO6)
>>> pin 10 (MIO10)
>>> pin 11 (MIO11)
>>>
>>> group: spi1_ss_0_grp
>>> pin 9 (MIO9)
>>>
>>> group: spi1_ss_1_grp
>>> pin 8 (MIO8)
>>>
>>> group: spi1_ss_2_grp
>>> pin 7 (MIO7)
>>>
>>> However, we are not using the "upper" pins of the QSPI device.
>>> Therefore, these pins should not be included in the qspi0_0_grp. This
>>> stems from the driver placing all possible pins into a function's group,
>>> even though each pin can be muxed individially and it is not necessary
>>> to mux all pins for full functionality.
>>
>> Correct. These configurations were not consider at that time when code
>> was written. The same issue is there if you want to combine pins from
>> different groups. IIRC uart rx via MIOX and tx not from MIOX+1.
>>
>>>
>>> I think it would be better to have a single group for each pin:
>>>
>>> pinctrl_qspi_default: qspi-default {
>>> mux {
>>> groups = "mio0", "mio1", "mio2", "mio3", "mio4";
>>> function = "qspi0";
>>> };
>>>
>>> mux-cs {
>>> groups = "mio5";
>>> function = "qspi_ss";
>>> };
>>> };
>>>
>>> pinctrl_spi1_default: spi1-default {
>>> mux {
>>> groups = "mio6", "mio10", "mio11";
>>> function = "spi1";
>>> };
>>>
>>> mux-cs {
>>> groups = "mio8", "mio9";
>>> function = "spi1_ss";
>>> };
>>> };
>>>
>>> This allows the full functionality of this chip to be configured. Does
>>> that sound good? I can send a patch to this effect if you agree.
>>
>> The only question is if this can be done without changing TF-A code
>> because we are running out of space in OCM for it.
>
> I think this can be done just in the Linux driver. This would also be
> convenient because then it will work regardless of the firmware.

Fine by me.

>> Just a generic question to your problem. It doesn't sound like a
>> dynamic case. You have static assignment for pins which likely won't
>> change over lifecycle. QSPI can be even boot device. Do you really
>> need to describe pins via DT that it is not enough to have them
>> configured via psu_init directly?
>>
>> Driver has been developed for i2c bus recovery via gpio which was the
>> main application. Right now Kria SOM is using it for carrier card pins
>> configuration. And Kria is pretty much only platform where this is
>> regularly tested.
>
> This is just following the example of the ZCU102. But you're right,
> these pin configurations are static (excepting I2C).

If this is going to product I would try to avoid communication as much as
possible because in static case it is not needed and just adding overhead to
your boot up time. But if you want to still do it I have no problem with it.

Thanks,
Michal