| Message ID | 20200921142655.v3.1.I997a428f58ef9d48b37a27a028360f34e66c00ec@changeid (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 37dd4b7779424cc7e26791c24e9918c52e01e1ee |
| Headers | show |
| Series | [v3,1/2] arm64: dts: qcom: sc7180: Provide pinconf for SPI to use GPIO for CS | expand |
On 9/22/2020 2:57 AM, Douglas Anderson wrote: > When the chip select line is controlled by the QUP, changing CS is a > time consuming operation. We have to send a command over to the geni > and wait for it to Ack us every time we want to change (both making it > high and low). To send this command we have to make a choice in > software when we want to control the chip select, we have to either: > A) Wait for the Ack via interrupt which slows down all SPI transfers > (and incurrs extra processing associated with interrupts). > B) Sit in a loop and poll, waiting for the Ack. > > Neither A) nor B) is a great option. > > We can avoid all of this by realizing that, at least on some boards, > there is no advantage of considering this line to be a geni line. > While it's true that geni _can_ control the line, it's also true that > the line can be a GPIO and there is no downside of viewing it that > way. Setting a GPIO is a simple MMIO operation. > > This patch provides definitions so a board can easily select the GPIO > mode. > > NOTE: apparently, it's possible to run the geni in "GSI" mode. In GSI > the SPI port is allowed to be controlled by more than one user (like > firmware and Linux) and also the port can operate sequences of > operations in one go. In GSI mode it _would_ be invalid to look at > the chip select as a GPIO because that would prevent other users from > using it. In theory GSI mode would also avoid some overhead by > allowing us to sequence the chip select better. However, I'll argue > GSI is not relevant for all boards (and certainly not any boards > supported by mainline today). Why? > - Apparently to run a SPI chip in GSI mode you need to initialize it > (in the bootloader) with a different firmware and then it will > always run in GSI mode. Since there is no support for GSI mode in > the current Linux driver, it must be that existing boards don't have > firmware that's doing that. Note that the kernel device tree > describes hardware but also firmware, so it is legitimate to make > the assumption that we don't have GSI firmware in a given dts file. > - Some boards with sc7180 have SPI connected to the Chrome OS EC or > security chip (Cr50). The protocols for talking to cros_ec and cr50 > are extremely complex. Both drivers in Linux fully lock the bus > across several distinct SPI transfers. While I am not an expert on > GSI mode it feels highly unlikely to me that we'd ever be able to > enable GSI mode for these devices. > > From a testing perspective, running "flashrom -p ec -r /tmp/foo.bin" > in a loop after this patch shows almost no reduction in time, but the > number of interrupts per command goes from 32357 down to 30611 (about > a 5% reduction). > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > Reviewed-by: Stephen Boyd <swboyd@chromium.org> > --- > > Reviewed-by: Akash Asthana <akashast@codeaurora.org>
Hi, On Mon, Sep 21, 2020 at 2:27 PM Douglas Anderson <dianders@chromium.org> wrote: > > When the chip select line is controlled by the QUP, changing CS is a > time consuming operation. We have to send a command over to the geni > and wait for it to Ack us every time we want to change (both making it > high and low). To send this command we have to make a choice in > software when we want to control the chip select, we have to either: > A) Wait for the Ack via interrupt which slows down all SPI transfers > (and incurrs extra processing associated with interrupts). > B) Sit in a loop and poll, waiting for the Ack. > > Neither A) nor B) is a great option. > > We can avoid all of this by realizing that, at least on some boards, > there is no advantage of considering this line to be a geni line. > While it's true that geni _can_ control the line, it's also true that > the line can be a GPIO and there is no downside of viewing it that > way. Setting a GPIO is a simple MMIO operation. > > This patch provides definitions so a board can easily select the GPIO > mode. > > NOTE: apparently, it's possible to run the geni in "GSI" mode. In GSI > the SPI port is allowed to be controlled by more than one user (like > firmware and Linux) and also the port can operate sequences of > operations in one go. In GSI mode it _would_ be invalid to look at > the chip select as a GPIO because that would prevent other users from > using it. In theory GSI mode would also avoid some overhead by > allowing us to sequence the chip select better. However, I'll argue > GSI is not relevant for all boards (and certainly not any boards > supported by mainline today). Why? > - Apparently to run a SPI chip in GSI mode you need to initialize it > (in the bootloader) with a different firmware and then it will > always run in GSI mode. Since there is no support for GSI mode in > the current Linux driver, it must be that existing boards don't have > firmware that's doing that. Note that the kernel device tree > describes hardware but also firmware, so it is legitimate to make > the assumption that we don't have GSI firmware in a given dts file. > - Some boards with sc7180 have SPI connected to the Chrome OS EC or > security chip (Cr50). The protocols for talking to cros_ec and cr50 > are extremely complex. Both drivers in Linux fully lock the bus > across several distinct SPI transfers. While I am not an expert on > GSI mode it feels highly unlikely to me that we'd ever be able to > enable GSI mode for these devices. > > From a testing perspective, running "flashrom -p ec -r /tmp/foo.bin" > in a loop after this patch shows almost no reduction in time, but the > number of interrupts per command goes from 32357 down to 30611 (about > a 5% reduction). > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > Reviewed-by: Stephen Boyd <swboyd@chromium.org> > --- > > Changes in v3: > - Add a blank line between pinmux and pinconf. > > Changes in v2: > - Now just add the pinctrl; let a board use it. > > arch/arm64/boot/dts/qcom/sc7180.dtsi | 104 +++++++++++++++++++++++++++ > 1 file changed, 104 insertions(+) Friendly ping that this patch and the next one in the series are reviewed and ready to land now that -rc1 is out. ;-) -Doug
diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi index 6678f1e8e395..73b1c17cfe7c 100644 --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi @@ -1595,6 +1595,19 @@ pinmux { }; }; + qup_spi0_cs_gpio: qup-spi0-cs-gpio { + pinmux { + pins = "gpio34", "gpio35", + "gpio36"; + function = "qup00"; + }; + + pinmux-cs { + pins = "gpio37"; + function = "gpio"; + }; + }; + qup_spi1_default: qup-spi1-default { pinmux { pins = "gpio0", "gpio1", @@ -1603,6 +1616,19 @@ pinmux { }; }; + qup_spi1_cs_gpio: qup-spi1-cs-gpio { + pinmux { + pins = "gpio0", "gpio1", + "gpio2"; + function = "qup01"; + }; + + pinmux-cs { + pins = "gpio3"; + function = "gpio"; + }; + }; + qup_spi3_default: qup-spi3-default { pinmux { pins = "gpio38", "gpio39", @@ -1611,6 +1637,19 @@ pinmux { }; }; + qup_spi3_cs_gpio: qup-spi3-cs-gpio { + pinmux { + pins = "gpio38", "gpio39", + "gpio40"; + function = "qup03"; + }; + + pinmux-cs { + pins = "gpio41"; + function = "gpio"; + }; + }; + qup_spi5_default: qup-spi5-default { pinmux { pins = "gpio25", "gpio26", @@ -1619,6 +1658,19 @@ pinmux { }; }; + qup_spi5_cs_gpio: qup-spi5-cs-gpio { + pinmux { + pins = "gpio25", "gpio26", + "gpio27"; + function = "qup05"; + }; + + pinmux-cs { + pins = "gpio28"; + function = "gpio"; + }; + }; + qup_spi6_default: qup-spi6-default { pinmux { pins = "gpio59", "gpio60", @@ -1627,6 +1679,19 @@ pinmux { }; }; + qup_spi6_cs_gpio: qup-spi6-cs-gpio { + pinmux { + pins = "gpio59", "gpio60", + "gpio61"; + function = "qup10"; + }; + + pinmux-cs { + pins = "gpio62"; + function = "gpio"; + }; + }; + qup_spi8_default: qup-spi8-default { pinmux { pins = "gpio42", "gpio43", @@ -1635,6 +1700,19 @@ pinmux { }; }; + qup_spi8_cs_gpio: qup-spi8-cs-gpio { + pinmux { + pins = "gpio42", "gpio43", + "gpio44"; + function = "qup12"; + }; + + pinmux-cs { + pins = "gpio45"; + function = "gpio"; + }; + }; + qup_spi10_default: qup-spi10-default { pinmux { pins = "gpio86", "gpio87", @@ -1643,6 +1721,19 @@ pinmux { }; }; + qup_spi10_cs_gpio: qup-spi10-cs-gpio { + pinmux { + pins = "gpio86", "gpio87", + "gpio88"; + function = "qup14"; + }; + + pinmux-cs { + pins = "gpio89"; + function = "gpio"; + }; + }; + qup_spi11_default: qup-spi11-default { pinmux { pins = "gpio53", "gpio54", @@ -1651,6 +1742,19 @@ pinmux { }; }; + qup_spi11_cs_gpio: qup-spi11-cs-gpio { + pinmux { + pins = "gpio53", "gpio54", + "gpio55"; + function = "qup15"; + }; + + pinmux-cs { + pins = "gpio56"; + function = "gpio"; + }; + }; + qup_uart0_default: qup-uart0-default { pinmux { pins = "gpio34", "gpio35",