On 10/18/23 14:47, Manivannan Sadhasivam wrote:
On Wed, Sep 20, 2023 at 12:23:27PM +0200, Manivannan Sadhasivam wrote:
On Wed, Sep 20, 2023 at 01:27:59AM +0300, Dmitry Baryshkov wrote:
On Tue, 19 Sept 2023 at 15:08, Manivannan Sadhasivam <mani@xxxxxxxxxx> wrote:
On Fri, Sep 15, 2023 at 05:31:45AM +0300, Dmitry Baryshkov wrote:
On 11 September 2023 13:02:50 GMT+03:00, Can Guo <quic_cang@xxxxxxxxxxx> wrote:
On 9/11/2023 5:46 PM, Konrad Dybcio wrote:
On 11.09.2023 11:42, Can Guo wrote:
Hi Konrad,
On 9/11/2023 5:17 PM, Konrad Dybcio wrote:
On 11.09.2023 07:59, Can Guo wrote:
From: "Bao D. Nguyen" <quic_nguyenb@xxxxxxxxxxx>
Retrieve UFS device version from UFS host controller's spare register
which is populated by bootloader, and use the UFS device version together
with host controller's HW version to decide the proper power modes which
should be used to configure the UFS PHY.
That sounds a bit fishy.. is there no bootloader-independent
solution to that? Can't we bring in the code that the bootloader
uses to determine these values?
Konrad
Agree, it is.
All these complexities come from one request from PHY design team - power saving.
And to achieve power saving, Qualcomm UFS developers are requested to use the
lowest hanging PHY settings which can sustain the Max agreed HS Gear (btw host
and UFS device) during UFS's lifecycle in High Level OS, whereas the power saving
request does not apply to bootloader, which works for only a few seconds during
bootup. Hence, there is no such version detect code in bootloader - it just uses the
highest PHY settings to configure PHY, boot up UFS and put UFS device version in this
register.
First of all, your email client seems to be inserting 2 newlines
instead of 1. If you're using thunderbird, you may want to edit:
mail.identity.(default or your mail identity idx).default.compose_html
to `false`
and add that to your internal wiki page, as I see many @quic folks having
this issue.
Going back to the main topic, I don't think we understood each other.
The commit message states:
"Retrieve UFS device version from UFS host controller's spare register
which is populated by bootloader"
Which means the bootloader is able to somehow determine the value
that's in the spare register and write it there.
I'm asking whether we can take the logic behind this value and
move it to Linux so that we don't depend on the bootloader to
guarantee it (e.g. Chrome or some other devices with more exotic
fw may not work this way).
Konrad
There is no logic behind this value at all in bootloader, as I explained, after bootloader
initializes UFS, bootloader simply reads UFS's device version (the value you are referring)
and write it to the register. But in Linux kernel, we need (or want to know) this value
BEFORE we initialize UFS host controller (and UFS device).
Depending on the bootloader behaviour is not an option. For example the kernel might be started via kexec. Or via u-boot. Or grub. Or any other bootloader. So please duplicate the logic to read the UFS version instead.
As Can said, there is no logic in the bootloader. What it does it, after doing
the UFS initialization, it writes the agreed gear (between host and the device)
to this register. And in linux, we use that value to initialize the device
(i.e., not doing init based on the min gear).
But the important factor here is that, we use this gear value to program the PHY
init sequence. So if there is no hint from the bootloader, linux will program
the min phy sequence (G3/G4) and then once the gear scaling happens, it will
program the max phy sequence (G4/G5).
Now on recent platforms, the init sequences are not compatible with each other
i.e., once the min seq. is programmed, then before programming max seq. the
registers not common to both seq. should be programmed to default value. In
other words, min seq. specific registers should be reset to the default value.
Otherwise, there will be stability issues in the PHY.
I see nothing wrong with adding 'default' register programming to the
gear tables. If we have to reset them to the default values to switch
the PHY settings, these writes must be a part of the corresponding
tables.
Yep, that's what I initially proposed. But Qcom wanted to avoid the cost of
programming the reset tables in the PHY driver.
Can, could you please check if programming the additional sequence doesn't cause
any power/performance effect?
I'd like to simplify this conversion as there has been some misunderstanding.
First of all in linux, while probing the UFS device by the host controller, it
needs to use _some_ gear. So far we were using HS_G2 as that gear and using the
PHY init sequence of G3/G4 depending on the SoC. We do not need to use G2 init
sequence because, there are only 2 init sequences available for any SoC and
since the init sequences are backwards compatible, we mostly use the min init
sequence, G3/G4. Even though this incurs slight power consumption during boot,
the ufs host controller after probing the device will switch to max gear
supported by both entities. If that max is G4/G5, then the respective init
sequence will be programmed again.
Now the issue is, for the automotive usecases, switching the gears 2 times
during boot is affecting the boot KPI (Key Performance Inidicator). So the UFS
team came with the idea of populating a spare register in the bootloader with
the max gear info that the bootloader has already found out and using the same
in the linux for first time itself. This helps linux in using a single gear
during probe time.
This is what this patch is doing. If for some reason, that register is not
populated, then we default to the existing G2 gear and do init twice as the
driver is doing currently.
I hope this clarifies the intention of this patch.
Yes I understand this, but I am not sure if such tricks should make
it upstream.. They depend on specific firmware (unrelated to the hw
block itself) and only exist to improve boot times. If the firmware
requirement was not at play, I would have no issues with this.
Konrad
