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

[Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>]

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?


-- 
WBR, Volodymyr