Re: [RESEND PATCH 2/9] dt-bindings: arm64: bcmbca: Update BCM4908 description

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 07/22/2022 11:22 AM, Krzysztof Kozlowski wrote:
On 21/07/2022 21:35, William Zhang wrote:


On 07/21/2022 09:43 AM, Florian Fainelli wrote:
On 7/21/22 00:50, Rafał Miłecki wrote:
On 2022-07-21 09:36, Krzysztof Kozlowski wrote:
On 21/07/2022 09:13, Rafał Miłecki wrote:
That's better argument. But what's the benefit of adding generic
compatible? Devices cannot bind to it (it is too generic). Does it
describe the device anyhow? Imagine someone adding compatible
"brcm,all-soc-of-broadcom" - does it make any sense?



OK, I see it now. I can't think of any case of handling all devices
covered with suc a wide brcm,bcmbca binding.

Maybe there is some common part of a SoC which that generic compatible
would express?

Most archs don't use soc-wide generic compatible, because of reasons I
mentioned - no actual benefits for anyone from such compatible.

But there are exceptions. I fouun socfpga and apple. The apple sounds as
mistake to me, because the generic "apple,arm-platform" compatible looks
like covering all possible Apple ARM platforms. I think Apple ARM
designs in 20 years will not be compatible at all with current design,
so such broad compatible is not useful... but that's only my opinion.

Let's see if William / Broadcom guys can provide a valid argument for
the brcm,bcmbca.

It is common practice to provide a generic fallback compatible string for a given chip family/architecture and all of our existing in-tree (and out of tree for that matter) DTSes for Broadcom SoCs do that:

- Documentation/devicetree/bindings/arm/bcm/brcm,brcmstb.txt
- Documentation/devicetree/bindings/arm/bcm/brcm,ns2.yaml
- Documentation/devicetree/bindings/arm/bcm/brcm,nsp.yaml
- Documentation/devicetree/bindings/arm/bcm/brcm,cygnus.yaml

list goes on and on, of course the counter examples are bcm2835, bcm4708 etc. although those are both chip and families technically, so I suppose the conflation is appropriate in that case. So the pattern is simple:

- outside of Broadcom contributors used convention
- inside of Broadcom contributors used another

so if nothing else, we ought to be consistent within ourselves as Broadcom insiders, which we are doing here.

While the generic fallback may not be in use, it still serves a purpose:

- Broadcom likes to create a gazillion of part numbers that are hard to untangle from their original SoC architecture unless you happen to work there so it serves as documentation for others to identify what family they belong to, and what to expect from that

- you never know when you might want to be matching on just the generic compatible string of a family and putting it in there costs nothing at all

The point of William's patch series is to right a number of wrongs on Broadcom's side:

- lack of appropriate involvements at the time Rafal submitted the 4908 support as a "standalone" family, I am to take the blame for suggesting that name in the first place, though I did not know at the time that William and others would ever be contributing upstream

- avoid the proliferation of "sub" families within a larger family (BCMBCA) since that serves no purpose other than to make it harder on users to select what they should be selecting in their kernel *and* it makes us inconsistent with arch/arm64/Kconfig.platforms that attempts to reduce those

I would conclude by asking you: why is this such a big issue? What *actual* problem does it causes, except maybe setting a precedent that you do not like, but for which you practically should have no reason to care as long as the binding is enforced.

Totally agreed.  Just want to emphasize that the main reason of this
change to merge all the bcmbca family chips into the same group for
better organization and reduce the clutter to the kernel source. Think
about 18 bcmbca devices with different changes to yaml, kconfig,
makefile, sub-directory while they actually share many common blocks,
drivers and dts entries.

Your commit does not explain why you are doing it at all. We have all
this discussion, you put so many arguments, but why none of it is in the
commit description?

The purpose of the discussion was to explain to why we think appending
brcm,bcmbca is necessary and helpful and the history of this string when
we first upstream the bcmbca SoC. And I also said I totally agree to the
improvement of commit message. I just posted the v2 of this patch series
with all the requests accommodated from this discussion.


While nobody would bind a device to brcm,bcmbca (and it can not be
binded because bcmbca,yaml require to prefix with a specific chip), it
is a great way to make it easy for people to understand what device they
are working and look for the bcmbca common driver and other support code
as well. Wouldn't this be good thing to have?

And don't forget we introduced bcmbca awhile back with first SoC 47622
and have set a clear goal for the purpose we are discussing here today:
Please see this patch series:
http://lists.infradead.org/pipermail/linux-arm-kernel/2022-April/732867.html
"This change introduces Broadcom's ARCH_BCMBCA architecture for
armv7 and armv8 based Broadband SoCs. We expect to send additional
patches for each SoC in the near future."

And Krzysztof acked that yaml for brcm,bcabca here :
http://lists.infradead.org/pipermail/linux-arm-kernel/2022-April/732867.html

and I keep discussing, so bringing it up proves what? That new comments
appeared?





Best regards,
Krzysztof

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature


[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux