Re: [PATCH v2 8/8] board: add support for Qualcomm SA8155P-ADP board

[Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>]

Caleb Connolly <caleb.connolly@linaro.org> writes:

> On 11/03/2024 18:23, Volodymyr Babchuk wrote:
>> Hi Caleb,
>> Caleb Connolly <caleb.connolly@linaro.org> writes:
>> 
>>> On 06/03/2024 21:24, Volodymyr Babchuk wrote:
>>>>
>>>> Hi Caleb,
>>>>
>>>> Caleb Connolly <caleb.connolly@linaro.org> writes:
>>>>
>>>> [...]
>>>>>>>> +};
>>>>>>>> +
>>>>>>>> +&tlmm {
>>>>>>>> +       /* U-Boot pinctrl driver does not understand multiple tiles */
>>>>>>>> +       reg = <0x0 0x03000000 0x0 0x1000000>;
>>>>>>>> +       /delete-property/ reg-names;
>>>>>>>
>>>>>>> This won't be needed if we can make the tiles offset in the pinctrl
>>>>>>> driver compatible:
>>>>>>>
>>>>>>> #define WEST   0x00000000
>>>>>>> #define EAST   0x00400000
>>>>>>> #define NORTH  0x00800000
>>>>>>> #define SOUTH  0x00C00000
>>>>>>
>>>>>> Hmm, I assume that in this case pinctrl driver should map all the four
>>>>>> tiles independently? Are there guarantees in U-Boot that four separate
>>>>>> memory regions will be mapped into virtual memory with the same relative
>>>>>> positions? Linux clearly don't make such guarantees.
>>>>>
>>>>> U-Boot doesn't use virtual addresses on arm platforms, it only goes as
>>>>> far as reading the address from DT, nothing else, so this is totally
>>>>> fine and is how the other SoCs do it.
>>>>
>>>> For me it looks like we are depending on implementation details
>>>> knowledge. I.e MMU API does not provide such guarantees, but drivers
>>>> know how ARM MMU code is working internally and drivers depend on
>>>> exactly this behavior. But if you are saying that it is totally fine,
>>>> I'll rework the patch. No big deal. Actually, I already tried this and
>>>> it is working fine.
>>>>
>>>>>>>> +
>>>>>>>> +       /* U-Boot ethernet driver wants to drive reset as GPIO */
>>>>>>>> +       /delete-node/ phy-reset-pins;
>>>>>>>
>>>>>>> I suppose this is not needed as phy-reset-pins also configures the pin
>>>>>>> as GPIO only.
>>>>>>>
>>>>>> Well, yes. This also puzzles me up, but for some reason it stops working
>>>>>> if I leave this node intact. Looks like I need to look at this deeper
>>>>>> before posting the next version.
>>>>>
>>>>> Possibly the pinconf defined in the phy-reset-pins node causes U-Boot to
>>>>> misbehave, can you check if this patch fixes it (there is a bug in the
>>>>> line "return msm_gpio_direction_input(dev, gpio);", it should become
>>>>> just "msm_gpio_direction_input(dev, gpio);").
>>>>>
>>>>> I had the exact same issue with the gpio-regulator driver and this was
>>>>> the solution I ended up going with.
>>>>>
>>>>> https://urldefense.com/v3/__https://lore.kernel.org/u-boot/20240131-b4-qcom-livetree-v1-7-4071c0787db0@linaro.org/__;!!GF_29dbcQIUBPA!xFhZe7DKgRbr63sirEJLuH-B0AnGs7jvx8tdJPKLTgFuZ3I3_zpVml7l23G-_vJO_JiUR-wUO4GMPJFcE-8p50H3pf7nbxit$
>>>>> [lore[.]kernel[.]org]
>>>>
>>>> It is exactly this. With your patch I don't need to /delete-node/
>>>> anymore. I'll add a comment in the cover message that this series are
>>>> depended on your patch.
>>>
>>> Please can you split the power domain and clock patches into a separate
>>> series? As I'd like to depend on them for the next revision of my
>>> series, and we'd otherwise have a cyclical dependency.
>> Of course.
>> As I understood, you are interested in "clk: qcom: clear div mask
>> before
>> assigning a new divider" and "clk: qcom: add support for power domains
>> uclass", correct?
>
> Yes.

Okay, I'll send it today.

> I tried the power domain stuff out on SMD845 today and ran into
> quite a few issues. Specifically as a lot of the devices reference the
> rpmhpd power domain which we don't support (and don't *need* to
> support) in U-Boot. I'm not sure what the best way forward will be for
> this. Maybe a "nop" power domain driver?

Are you sure that they are not required?

"nop" power domain always is the option. Especially if it prints some
warning about an unknown device. I had quite a lot of issues with clock and
pin drivers that silently ignore unknown devices...

> Do you have the same issues on sm8150?

Yes and no. No, because I was lucky so far and devices I tried to use in
U-Boot does not require rpmhpd. Looking at DTS, I may only encounter
issues with sdhc_2, which requires rpmhpd for some reason. Also UFS
requires clock from rpmhcc.

And "yes", because I have found root cause for my troubles with UFS in
Linux kernel, when I am skipping hyp.mbn. This is not strictly related
to U-Boot, but you may be interested in this: apparently Qualcomm's
hypervisor enables access to RPM (maybe brings it out of reset?). cmd-db
shared memory region can't be accessed if I skip the hypervisor and try
to boot directly into Linux. So now I am looking for ways to enable it.

-- 
WBR, Volodymyr