Re: [PATCH] timer: bcmbca: Add Broadcom BCMBCA timer support

[Florian Fainelli <f.fainelli@gmail.com>]

On 8/2/2022 5:07 PM, William Zhang wrote:
> Hi Rafal,
> 
> On 08/01/2022 10:26 PM, Rafał Miłecki wrote:
>> On 2.08.2022 00:03, William Zhang wrote:
>>> This driver supports the peripheral block timer found on the Broadcom
>>> BCA SoCs. It is 30-bit up-count timer running at 50MHz and can be used
>>> as the system clock source such as on BCM63138.
>>> Signed-off-by: William Zhang <william.zhang@broadcom.com>
>>>
>>> (...)
>>>
>>> +static const struct udevice_id bcmbca_timer_ids[] = {
>>> +    { .compatible = "brcm,bcm-timers" },
>>> +    { }
>>> +};
>>> +
>>> +U_BOOT_DRIVER(bcmbca_timer) = {
>>> +    .name = "bcmbca_timer",
>>> +    .id = UCLASS_TIMER,
>>> +    .of_match = bcmbca_timer_ids,
>>> +    .priv_auto = sizeof(struct bcmbca_timer_priv),
>>> +    .probe = bcmbca_timer_probe,
>>> +    .ops = &bcmbca_timer_ops,
>>> +    .flags = DM_FLAG_PRE_RELOC,
>>> +};
>>
>> That "brcm,bcm-timers" seems like a really wide bidding. Is that exact
>> timer block guaranteed to be present on all Broadcom devices? Does it
>> exist e.g. on Northstar SoCs? Or old MIPS SoCs like BCM4706?
>>
> Agree I will make change to use brcm,bcmbca-timers.

Why not change to use a compatible string that uses the first chip in 
which this timer block was introduced, was that 6345 or later?

> 
>> It seems that even across BCMBCA devices this block may differ and may
>> need different bindings. Most SoCs have 4 CTL and 4 CNT registers but
>> some have only 3 + 3 (BCM6838 BCM60333 BCM63268).
> This timer driver is intended for the clock source for u-boot only so 
> only need the first channel. Although it could work for the old mips 
> based dsl chip, BCMBCA chip family support is only for the new ARM based 
>   chips which is stated in the BCMBA introduction patch.
> 
>>
>> Finally could we have that binding actually documented?
> I don't see u-boot has binding document.  I believe it generally use 
> linux binding document.  I have no plan to upstream this driver to 
> linux. But I can put the binding info in the driver if that is the right 
> way to do in u-boot.

Out of curiosity, why does u-boot require this timer as opposed to using 
the Cortex-A9 architected timers? Is it necessary to boot strap the A9 
timers?
-- 
Florian