Re: [PATCH v1 6/8] PCI: brcmstb: Don't conflate the reset rescal with phy ctrl

[Stanimir Varbanov <svarbanov@suse.de>]

On 7/2/24 20:59, Jim Quinlan wrote:
> On Tue, Jul 2, 2024 at 9:10 AM Stanimir Varbanov <svarbanov@suse.de> wrote:
>>
>>
>>
>> On 6/28/24 23:54, Jim Quinlan wrote:
>>> We've been assuming that if an SOC has a "rescal" reset controller that we
>>> should automatically invoke brcm_phy_cntl(...).  This will not be true in
>>> future SOCs, so we create a bool "has_phy" and adjust the cfg_data
>>> appropriately (we need to give 7216 its own cfg_data structure instead of
>>> sharing one).
>>>
>>> Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
>>> ---
>>>  drivers/pci/controller/pcie-brcmstb.c | 17 ++++++++++++++---
>>>  1 file changed, 14 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c

<cut>

>>>
>>> +static const struct pcie_cfg_data bcm7216_cfg = {
>>> +     .offsets        = pcie_offset_bcm7278,
>>> +     .type           = BCM7278,
>>
>> This "type" field is confusing, maybe it would be good to rename it to
>> "family"? For example BCM72XX family.
> 
> Hi Stanimir,
> 
> I'm open for another name but "family" would present problems with Broadcom STB.
> For example, we call 7216b0 a "family" as there are a number of
> derivative products based off

OK, sorry I'm not familiar with STB families. Then, it makes sense.

> of this general design.  Second, having something like "BCM72XX" won't work;
> we have 7211 which is something altogether different from the 7216.
> Note that we only
> introduce a new "type" when we need to; if the behavior is the same as
> a previously declared
> "type" we do not introduce new ones.
> 
> But if you wanted to change "type" to "model" then I have no problem with that.
> 

"model" sounds good to me. We might need to document this in kernel doc
style comment in struct pcie_cfg_data as a separate patch.

> Regards,
> Jim Quinlan

thanks
~Stan