| Message ID | 20231122144208.21114-1-zajec5@gmail.com |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | [V2,1/2] dt-bindings: serial: add Broadcom's BCMBCA family High Speed UART | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On 22/11/2023 15:42, Rafał Miłecki wrote: > From: Rafał Miłecki <rafal@milecki.pl> > > It's an UART controller that first appeared on BCM63138 SoC and then was > reused on other bcmbca familiy chipsets. If there is going to be a new version, typo: family > > Signed-off-by: Rafał Miłecki <rafal@milecki.pl> > --- > V2: Extend "compatible" and rename YAML file accordingly > > Krzysztof: since I reworked "compatible" I didn't want to carry on your > Reviewed in case there is sth wrong with the updated schema. Thanks for letting me know. > + > +maintainers: > + - Rafał Miłecki <rafal@milecki.pl> > + > +allOf: > + - $ref: serial.yaml# > + > +properties: > + compatible: > + items: > + - enum: > + - brcm,bcm4908-hs-uart > + - brcm,bcm4912-hs-uart > + - brcm,bcm6756-hs-uart > + - brcm,bcm6813-hs-uart > + - brcm,bcm6846-hs-uart > + - brcm,bcm6855-hs-uart > + - brcm,bcm6856-hs-uart > + - brcm,bcm6858-hs-uart > + - brcm,bcm6878-hs-uart > + - brcm,bcm47622-hs-uart > + - brcm,bcm63138-hs-uart > + - brcm,bcm63146-hs-uart > + - brcm,bcm63158-hs-uart > + - brcm,bcm63178-hs-uart > + - const: brcm,bcmbca-hs-uart git grep did not find driver for this compatible. Is it in separate patchset? Best regards, Krzysztof
On 22.11.2023 15:46, Krzysztof Kozlowski wrote: > On 22/11/2023 15:42, Rafał Miłecki wrote: >> From: Rafał Miłecki <rafal@milecki.pl> >> >> It's an UART controller that first appeared on BCM63138 SoC and then was >> reused on other bcmbca familiy chipsets. > > If there is going to be a new version, typo: family > >> >> Signed-off-by: Rafał Miłecki <rafal@milecki.pl> >> --- >> V2: Extend "compatible" and rename YAML file accordingly >> >> Krzysztof: since I reworked "compatible" I didn't want to carry on your >> Reviewed in case there is sth wrong with the updated schema. > > Thanks for letting me know. > >> + >> +maintainers: >> + - Rafał Miłecki <rafal@milecki.pl> >> + >> +allOf: >> + - $ref: serial.yaml# >> + >> +properties: >> + compatible: >> + items: >> + - enum: >> + - brcm,bcm4908-hs-uart >> + - brcm,bcm4912-hs-uart >> + - brcm,bcm6756-hs-uart >> + - brcm,bcm6813-hs-uart >> + - brcm,bcm6846-hs-uart >> + - brcm,bcm6855-hs-uart >> + - brcm,bcm6856-hs-uart >> + - brcm,bcm6858-hs-uart >> + - brcm,bcm6878-hs-uart >> + - brcm,bcm47622-hs-uart >> + - brcm,bcm63138-hs-uart >> + - brcm,bcm63146-hs-uart >> + - brcm,bcm63158-hs-uart >> + - brcm,bcm63178-hs-uart >> + - const: brcm,bcmbca-hs-uart > > git grep did not find driver for this compatible. Is it in separate > patchset? No. My project based on BCMBCA has been canceled and I don't work on it full time anymore. I just wanted to fill empty bits I can afford handling in my free time and complete hardware description in DTS. I may still work on some BCMBCA drivers from time to time but as a side project.
On 22/11/2023 15:52, Rafał Miłecki wrote: >>> +maintainers: >>> + - Rafał Miłecki <rafal@milecki.pl> >>> + >>> +allOf: >>> + - $ref: serial.yaml# >>> + >>> +properties: >>> + compatible: >>> + items: >>> + - enum: >>> + - brcm,bcm4908-hs-uart >>> + - brcm,bcm4912-hs-uart >>> + - brcm,bcm6756-hs-uart >>> + - brcm,bcm6813-hs-uart >>> + - brcm,bcm6846-hs-uart >>> + - brcm,bcm6855-hs-uart >>> + - brcm,bcm6856-hs-uart >>> + - brcm,bcm6858-hs-uart >>> + - brcm,bcm6878-hs-uart >>> + - brcm,bcm47622-hs-uart >>> + - brcm,bcm63138-hs-uart >>> + - brcm,bcm63146-hs-uart >>> + - brcm,bcm63158-hs-uart >>> + - brcm,bcm63178-hs-uart >>> + - const: brcm,bcmbca-hs-uart >> >> git grep did not find driver for this compatible. Is it in separate >> patchset? > > No. My project based on BCMBCA has been canceled and I don't work on it > full time anymore. I just wanted to fill empty bits I can afford > handling in my free time and complete hardware description in DTS. > > I may still work on some BCMBCA drivers from time to time but as a side > project. This means we cannot use driver to verify whether the fallback is actually suitable. Considering that existing UART bindings do not fallback (brcm,bcm6345-uart, brcm,bcm7271-uart), I don't understand what is the benefit here. Best regards, Krzysztof
On 22.11.2023 16:00, Krzysztof Kozlowski wrote: > On 22/11/2023 15:52, Rafał Miłecki wrote: >>>> +maintainers: >>>> + - Rafał Miłecki <rafal@milecki.pl> >>>> + >>>> +allOf: >>>> + - $ref: serial.yaml# >>>> + >>>> +properties: >>>> + compatible: >>>> + items: >>>> + - enum: >>>> + - brcm,bcm4908-hs-uart >>>> + - brcm,bcm4912-hs-uart >>>> + - brcm,bcm6756-hs-uart >>>> + - brcm,bcm6813-hs-uart >>>> + - brcm,bcm6846-hs-uart >>>> + - brcm,bcm6855-hs-uart >>>> + - brcm,bcm6856-hs-uart >>>> + - brcm,bcm6858-hs-uart >>>> + - brcm,bcm6878-hs-uart >>>> + - brcm,bcm47622-hs-uart >>>> + - brcm,bcm63138-hs-uart >>>> + - brcm,bcm63146-hs-uart >>>> + - brcm,bcm63158-hs-uart >>>> + - brcm,bcm63178-hs-uart >>>> + - const: brcm,bcmbca-hs-uart >>> >>> git grep did not find driver for this compatible. Is it in separate >>> patchset? >> >> No. My project based on BCMBCA has been canceled and I don't work on it >> full time anymore. I just wanted to fill empty bits I can afford >> handling in my free time and complete hardware description in DTS. >> >> I may still work on some BCMBCA drivers from time to time but as a side >> project. > > This means we cannot use driver to verify whether the fallback is > actually suitable. Considering that existing UART bindings do not > fallback (brcm,bcm6345-uart, brcm,bcm7271-uart), I don't understand what > is the benefit here. I believed the rule for maintaining bindings and DTS files was to describe hardware no matter what/if system needs it. For example a year ago I added binding for BCMBCA SoC timer without actual driver, see e112f2de151b ("dt-bindings: timer: Add Broadcom's BCMBCA timers"). I'm not sure if we're going to agree on this, but personally I like describing hardware as much as I can. So it's well documented / understood and people may eventually write drivers for it. Maybe it's partially because I come from Broadcom's world that isn't well known for upstream efforts in general. As for verifying this binding against actual driver I can definitely understand your concerns. Hoping it may help I uploaded Broadcom's HS UART driver extracted from the RAXE500-V1.0.8.70_2.0.36_gpl SDK/GPL package: http://files.zajec.net/hs_uart/ Please note it's not much of a clean code and its design would not be accepted upstream but hopefully you can glance at it to verify this binding's compatibility. Let me know if there is anything else (other than rewriting Broadcom's downstream driver) I could do to get this binding accepted.
On 22/11/2023 16:32, Rafał Miłecki wrote: > On 22.11.2023 16:00, Krzysztof Kozlowski wrote: >> On 22/11/2023 15:52, Rafał Miłecki wrote: >>>>> +maintainers: >>>>> + - Rafał Miłecki <rafal@milecki.pl> >>>>> + >>>>> +allOf: >>>>> + - $ref: serial.yaml# >>>>> + >>>>> +properties: >>>>> + compatible: >>>>> + items: >>>>> + - enum: >>>>> + - brcm,bcm4908-hs-uart >>>>> + - brcm,bcm4912-hs-uart >>>>> + - brcm,bcm6756-hs-uart >>>>> + - brcm,bcm6813-hs-uart >>>>> + - brcm,bcm6846-hs-uart >>>>> + - brcm,bcm6855-hs-uart >>>>> + - brcm,bcm6856-hs-uart >>>>> + - brcm,bcm6858-hs-uart >>>>> + - brcm,bcm6878-hs-uart >>>>> + - brcm,bcm47622-hs-uart >>>>> + - brcm,bcm63138-hs-uart >>>>> + - brcm,bcm63146-hs-uart >>>>> + - brcm,bcm63158-hs-uart >>>>> + - brcm,bcm63178-hs-uart >>>>> + - const: brcm,bcmbca-hs-uart >>>> >>>> git grep did not find driver for this compatible. Is it in separate >>>> patchset? >>> >>> No. My project based on BCMBCA has been canceled and I don't work on it >>> full time anymore. I just wanted to fill empty bits I can afford >>> handling in my free time and complete hardware description in DTS. >>> >>> I may still work on some BCMBCA drivers from time to time but as a side >>> project. >> >> This means we cannot use driver to verify whether the fallback is >> actually suitable. Considering that existing UART bindings do not >> fallback (brcm,bcm6345-uart, brcm,bcm7271-uart), I don't understand what >> is the benefit here. > > I believed the rule for maintaining bindings and DTS files was to > describe hardware no matter what/if system needs it. > > For example a year ago I added binding for BCMBCA SoC timer without > actual driver, see e112f2de151b ("dt-bindings: timer: Add Broadcom's > BCMBCA timers"). > > I'm not sure if we're going to agree on this, but personally I like > describing hardware as much as I can. So it's well documented / > understood and people may eventually write drivers for it. Maybe it's > partially because I come from Broadcom's world that isn't well known > for upstream efforts in general. The problem is that "brcm,bcmbca-hs-uart" is not describing hardware. It is saying that all these devices have similar (compatible) programming model, so the OS can use just one compatible. This goes away from pure hardware description into interpretation. Rob already commented on such non-SoC compatibles multiple times. I do not see any reason here to not use specific compatible as fallback. > > As for verifying this binding against actual driver I can definitely > understand your concerns. Hoping it may help I uploaded Broadcom's HS > UART driver extracted from the RAXE500-V1.0.8.70_2.0.36_gpl SDK/GPL > package: http://files.zajec.net/hs_uart/ > > Please note it's not much of a clean code and its design would not be > accepted upstream but hopefully you can glance at it to verify this > binding's compatibility. > > Let me know if there is anything else (other than rewriting Broadcom's > downstream driver) I could do to get this binding accepted. Best regards, Krzysztof
On 22.11.2023 16:37, Krzysztof Kozlowski wrote: > On 22/11/2023 16:32, Rafał Miłecki wrote: >> On 22.11.2023 16:00, Krzysztof Kozlowski wrote: >>> On 22/11/2023 15:52, Rafał Miłecki wrote: >>>>>> +maintainers: >>>>>> + - Rafał Miłecki <rafal@milecki.pl> >>>>>> + >>>>>> +allOf: >>>>>> + - $ref: serial.yaml# >>>>>> + >>>>>> +properties: >>>>>> + compatible: >>>>>> + items: >>>>>> + - enum: >>>>>> + - brcm,bcm4908-hs-uart >>>>>> + - brcm,bcm4912-hs-uart >>>>>> + - brcm,bcm6756-hs-uart >>>>>> + - brcm,bcm6813-hs-uart >>>>>> + - brcm,bcm6846-hs-uart >>>>>> + - brcm,bcm6855-hs-uart >>>>>> + - brcm,bcm6856-hs-uart >>>>>> + - brcm,bcm6858-hs-uart >>>>>> + - brcm,bcm6878-hs-uart >>>>>> + - brcm,bcm47622-hs-uart >>>>>> + - brcm,bcm63138-hs-uart >>>>>> + - brcm,bcm63146-hs-uart >>>>>> + - brcm,bcm63158-hs-uart >>>>>> + - brcm,bcm63178-hs-uart >>>>>> + - const: brcm,bcmbca-hs-uart >>>>> >>>>> git grep did not find driver for this compatible. Is it in separate >>>>> patchset? >>>> >>>> No. My project based on BCMBCA has been canceled and I don't work on it >>>> full time anymore. I just wanted to fill empty bits I can afford >>>> handling in my free time and complete hardware description in DTS. >>>> >>>> I may still work on some BCMBCA drivers from time to time but as a side >>>> project. >>> >>> This means we cannot use driver to verify whether the fallback is >>> actually suitable. Considering that existing UART bindings do not >>> fallback (brcm,bcm6345-uart, brcm,bcm7271-uart), I don't understand what >>> is the benefit here. >> >> I believed the rule for maintaining bindings and DTS files was to >> describe hardware no matter what/if system needs it. >> >> For example a year ago I added binding for BCMBCA SoC timer without >> actual driver, see e112f2de151b ("dt-bindings: timer: Add Broadcom's >> BCMBCA timers"). >> >> I'm not sure if we're going to agree on this, but personally I like >> describing hardware as much as I can. So it's well documented / >> understood and people may eventually write drivers for it. Maybe it's >> partially because I come from Broadcom's world that isn't well known >> for upstream efforts in general. > > The problem is that "brcm,bcmbca-hs-uart" is not describing hardware. It > is saying that all these devices have similar (compatible) programming > model, so the OS can use just one compatible. This goes away from pure > hardware description into interpretation. > > Rob already commented on such non-SoC compatibles multiple times. I do > not see any reason here to not use specific compatible as fallback. Do I get it right we should rather have some base specific compatible like: "brcm,bcm63138-hs-uart" and then if anything use fallback to it like: "brcm,bcm4908-hs-uart", "brcm,bcm63138-hs-uart"; ?
On 22/11/2023 16:49, Rafał Miłecki wrote: >>> For example a year ago I added binding for BCMBCA SoC timer without >>> actual driver, see e112f2de151b ("dt-bindings: timer: Add Broadcom's >>> BCMBCA timers"). >>> >>> I'm not sure if we're going to agree on this, but personally I like >>> describing hardware as much as I can. So it's well documented / >>> understood and people may eventually write drivers for it. Maybe it's >>> partially because I come from Broadcom's world that isn't well known >>> for upstream efforts in general. >> >> The problem is that "brcm,bcmbca-hs-uart" is not describing hardware. It >> is saying that all these devices have similar (compatible) programming >> model, so the OS can use just one compatible. This goes away from pure >> hardware description into interpretation. >> >> Rob already commented on such non-SoC compatibles multiple times. I do >> not see any reason here to not use specific compatible as fallback. > > Do I get it right we should rather have some base specific compatible > like: "brcm,bcm63138-hs-uart" and then if anything use fallback to it > like: "brcm,bcm4908-hs-uart", "brcm,bcm63138-hs-uart"; ? Yes, or the other way around, depends which is probably the oldest. Best regards, Krzysztof
On 22.11.2023 16:50, Krzysztof Kozlowski wrote: > On 22/11/2023 16:49, Rafał Miłecki wrote: >>>> For example a year ago I added binding for BCMBCA SoC timer without >>>> actual driver, see e112f2de151b ("dt-bindings: timer: Add Broadcom's >>>> BCMBCA timers"). >>>> >>>> I'm not sure if we're going to agree on this, but personally I like >>>> describing hardware as much as I can. So it's well documented / >>>> understood and people may eventually write drivers for it. Maybe it's >>>> partially because I come from Broadcom's world that isn't well known >>>> for upstream efforts in general. >>> >>> The problem is that "brcm,bcmbca-hs-uart" is not describing hardware. It >>> is saying that all these devices have similar (compatible) programming >>> model, so the OS can use just one compatible. This goes away from pure >>> hardware description into interpretation. >>> >>> Rob already commented on such non-SoC compatibles multiple times. I do >>> not see any reason here to not use specific compatible as fallback. >> >> Do I get it right we should rather have some base specific compatible >> like: "brcm,bcm63138-hs-uart" and then if anything use fallback to it >> like: "brcm,bcm4908-hs-uart", "brcm,bcm63138-hs-uart"; ? > > Yes, or the other way around, depends which is probably the oldest. Thank you for helping me on that!
Hi, On 11/22/2023 07:52 AM, Rafał Miłecki wrote: > On 22.11.2023 16:50, Krzysztof Kozlowski wrote: >> On 22/11/2023 16:49, Rafał Miłecki wrote: >>>>> For example a year ago I added binding for BCMBCA SoC timer without >>>>> actual driver, see e112f2de151b ("dt-bindings: timer: Add Broadcom's >>>>> BCMBCA timers"). >>>>> >>>>> I'm not sure if we're going to agree on this, but personally I like >>>>> describing hardware as much as I can. So it's well documented / >>>>> understood and people may eventually write drivers for it. Maybe it's >>>>> partially because I come from Broadcom's world that isn't well known >>>>> for upstream efforts in general. >>>> >>>> The problem is that "brcm,bcmbca-hs-uart" is not describing >>>> hardware. It >>>> is saying that all these devices have similar (compatible) programming >>>> model, so the OS can use just one compatible. This goes away from pure >>>> hardware description into interpretation. >>>> It is the same hardware IP block used in bcmbca SoCs. To me, it perfectly describe the hardware IP block and it does not need fallback because there is no fallback. We did that for SPI controller although it has two revisions of that IP block so we have brcm,bcmbca-hsspi-v1.0 and 1.1 >>>> Rob already commented on such non-SoC compatibles multiple times. I do >>>> not see any reason here to not use specific compatible as fallback. >>> Sorry I missed Rob's comments. If we have any new rule or notes about this, I would like to check it out. >>> Do I get it right we should rather have some base specific compatible >>> like: "brcm,bcm63138-hs-uart" and then if anything use fallback to it >>> like: "brcm,bcm4908-hs-uart", "brcm,bcm63138-hs-uart"; ? >> >> Yes, or the other way around, depends which is probably the oldest. If we absolutely can not use bcmbca-hs-uart, I would suggest to use bcm63xx-hs-uart to be more soc specific and in fact the oldest SoC have such IP is 63138. > > Thank you for helping me on that!
On 22/11/2023 19:39, William Zhang wrote: > Hi, > > On 11/22/2023 07:52 AM, Rafał Miłecki wrote: >> On 22.11.2023 16:50, Krzysztof Kozlowski wrote: >>> On 22/11/2023 16:49, Rafał Miłecki wrote: >>>>>> For example a year ago I added binding for BCMBCA SoC timer without >>>>>> actual driver, see e112f2de151b ("dt-bindings: timer: Add Broadcom's >>>>>> BCMBCA timers"). >>>>>> >>>>>> I'm not sure if we're going to agree on this, but personally I like >>>>>> describing hardware as much as I can. So it's well documented / >>>>>> understood and people may eventually write drivers for it. Maybe it's >>>>>> partially because I come from Broadcom's world that isn't well known >>>>>> for upstream efforts in general. >>>>> >>>>> The problem is that "brcm,bcmbca-hs-uart" is not describing >>>>> hardware. It >>>>> is saying that all these devices have similar (compatible) programming >>>>> model, so the OS can use just one compatible. This goes away from pure >>>>> hardware description into interpretation. >>>>> > It is the same hardware IP block used in bcmbca SoCs. To me, it > perfectly describe the hardware IP block and it does not need fallback > because there is no fallback. We did that for SPI controller although > it has two revisions of that IP block so we have brcm,bcmbca-hsspi-v1.0 > and 1.1 > >>>>> Rob already commented on such non-SoC compatibles multiple times. I do >>>>> not see any reason here to not use specific compatible as fallback. >>>> > Sorry I missed Rob's comments. If we have any new rule or notes about > this, I would like to check it out. > >>>> Do I get it right we should rather have some base specific compatible >>>> like: "brcm,bcm63138-hs-uart" and then if anything use fallback to it >>>> like: "brcm,bcm4908-hs-uart", "brcm,bcm63138-hs-uart"; ? >>> >>> Yes, or the other way around, depends which is probably the oldest. > If we absolutely can not use bcmbca-hs-uart, I would suggest to use We can, but I am surprised that you want without any driver. What's the point of generic compatible? > bcm63xx-hs-uart to be more soc specific and in fact the oldest SoC have What is xx? Wildcard? I mean... ehhh... Best regards, Krzysztof
On 22/11/2023 19:46, Krzysztof Kozlowski wrote: > On 22/11/2023 19:39, William Zhang wrote: >> Hi, >> >> On 11/22/2023 07:52 AM, Rafał Miłecki wrote: >>> On 22.11.2023 16:50, Krzysztof Kozlowski wrote: >>>> On 22/11/2023 16:49, Rafał Miłecki wrote: >>>>>>> For example a year ago I added binding for BCMBCA SoC timer without >>>>>>> actual driver, see e112f2de151b ("dt-bindings: timer: Add Broadcom's >>>>>>> BCMBCA timers"). >>>>>>> >>>>>>> I'm not sure if we're going to agree on this, but personally I like >>>>>>> describing hardware as much as I can. So it's well documented / >>>>>>> understood and people may eventually write drivers for it. Maybe it's >>>>>>> partially because I come from Broadcom's world that isn't well known >>>>>>> for upstream efforts in general. >>>>>> >>>>>> The problem is that "brcm,bcmbca-hs-uart" is not describing >>>>>> hardware. It >>>>>> is saying that all these devices have similar (compatible) programming >>>>>> model, so the OS can use just one compatible. This goes away from pure >>>>>> hardware description into interpretation. >>>>>> >> It is the same hardware IP block used in bcmbca SoCs. To me, it >> perfectly describe the hardware IP block and it does not need fallback >> because there is no fallback. We did that for SPI controller although >> it has two revisions of that IP block so we have brcm,bcmbca-hsspi-v1.0 >> and 1.1 >> >>>>>> Rob already commented on such non-SoC compatibles multiple times. I do >>>>>> not see any reason here to not use specific compatible as fallback. >>>>> >> Sorry I missed Rob's comments. If we have any new rule or notes about >> this, I would like to check it out. >> >>>>> Do I get it right we should rather have some base specific compatible >>>>> like: "brcm,bcm63138-hs-uart" and then if anything use fallback to it >>>>> like: "brcm,bcm4908-hs-uart", "brcm,bcm63138-hs-uart"; ? >>>> >>>> Yes, or the other way around, depends which is probably the oldest. >> If we absolutely can not use bcmbca-hs-uart, I would suggest to use > > We can, but I am surprised that you want without any driver. What's the > point of generic compatible? > >> bcm63xx-hs-uart to be more soc specific and in fact the oldest SoC have > > What is xx? Wildcard? I mean... ehhh... OK, it's not worth my time. Neither Rafał's. Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> I can go to Embedded OSS every year and give the same speech every year and still people will on: 1. insist on generic fallback compatible, 2. wildcards 3. families I will keep this email and use it to justify the same, third speech next year. Which won't be listened to, so I will go in 2025 fourth time. :) Best regards, Krzysztof
> -----Original Message----- > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Sent: Wednesday, November 22, 2023 10:46 AM > To: William Zhang <william.zhang@broadcom.com>; Rafał Miłecki > <zajec5@gmail.com>; Florian Fainelli <florian.fainelli@broadcom.com>; > Anand Gore <anand.gore@broadcom.com>; Kursad Oney > <kursad.oney@broadcom.com>; Rob Herring <robh+dt@kernel.org>; > Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley > <conor+dt@kernel.org> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Jiri Slaby > <jirislaby@kernel.org>; Andre Przywara <andre.przywara@arm.com>; > Alexandre TORGUE <alexandre.torgue@st.com>; Neil Armstrong > <neil.armstrong@linaro.org>; linux-serial@vger.kernel.org; > devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; bcm- > kernel-feedback-list@broadcom.com; Rafał Miłecki <rafal@milecki.pl> > Subject: Re: [PATCH V2 1/2] dt-bindings: serial: add Broadcom's BCMBCA > family High Speed UART > > On 22/11/2023 19:39, William Zhang wrote: > > Hi, > > > > On 11/22/2023 07:52 AM, Rafał Miłecki wrote: > >> On 22.11.2023 16:50, Krzysztof Kozlowski wrote: > >>> On 22/11/2023 16:49, Rafał Miłecki wrote: > >>>>>> For example a year ago I added binding for BCMBCA SoC timer > without > >>>>>> actual driver, see e112f2de151b ("dt-bindings: timer: Add > Broadcom's > >>>>>> BCMBCA timers"). > >>>>>> > >>>>>> I'm not sure if we're going to agree on this, but personally I like > >>>>>> describing hardware as much as I can. So it's well documented / > >>>>>> understood and people may eventually write drivers for it. Maybe > it's > >>>>>> partially because I come from Broadcom's world that isn't well > known > >>>>>> for upstream efforts in general. > >>>>> > >>>>> The problem is that "brcm,bcmbca-hs-uart" is not describing > >>>>> hardware. It > >>>>> is saying that all these devices have similar (compatible) > >>>>> programming > >>>>> model, so the OS can use just one compatible. This goes away from > pure > >>>>> hardware description into interpretation. > >>>>> > > It is the same hardware IP block used in bcmbca SoCs. To me, it > > perfectly describe the hardware IP block and it does not need fallback > > because there is no fallback. We did that for SPI controller although > > it has two revisions of that IP block so we have brcm,bcmbca-hsspi-v1.0 > > and 1.1 > > > >>>>> Rob already commented on such non-SoC compatibles multiple times. > I do > >>>>> not see any reason here to not use specific compatible as fallback. > >>>> > > Sorry I missed Rob's comments. If we have any new rule or notes about > > this, I would like to check it out. > > > >>>> Do I get it right we should rather have some base specific compatible > >>>> like: "brcm,bcm63138-hs-uart" and then if anything use fallback to it > >>>> like: "brcm,bcm4908-hs-uart", "brcm,bcm63138-hs-uart"; ? > >>> > >>> Yes, or the other way around, depends which is probably the oldest. > > If we absolutely can not use bcmbca-hs-uart, I would suggest to use > > We can, but I am surprised that you want without any driver. What's the > point of generic compatible? > I agree we should have the driver along with the dts. But it looks it depends on the bandwidth of Rafal. > > bcm63xx-hs-uart to be more soc specific and in fact the oldest SoC have > > What is xx? Wildcard? I mean... ehhh... > Bcm63 series of SoC (DSL broadband chip, part of the bcmbca family) and it is the oldest chip for such IP. bcm63xx has been used in many ip block's compatible string for long time in the upstream kernel. > Best regards, > Krzysztof
Howdy, On 11/22/2023 10:56 AM, Krzysztof Kozlowski wrote: >> >> What is xx? Wildcard? I mean... ehhh... > > OK, it's not worth my time. Neither Rafał's. Let me start of that I do get your point, but I also get William's. If you are going to use this email thread as proof that people should not use wildcards or families in compatible strings, this is very much worth everyone's time so we can actually detail better why that is an issue. So far it has been described as an inadequate description of the hardware which is somewhat fair, but subjective as well. The point of the fallback is precisely to express that the various HW IP versions have a similar programming model and that unless specified otherwise by a more descriptive compatible string, the driver can make that assumption. This is entirely within the spirit of the DT spec: """ The compatible property value consists of one or more strings that define the specific programming model for the device. This list of strings should be used by a client program for device driver selection. The property value consists of a concatenated list of null terminated strings, from most specific to most general. They allow a device to express its compatibility with a family of similar devices, potentially allowing a single device driver to match against several devices """ we simply differ from the recommendation, which is a recommendation, meaning deviation is allowed, and even that is entirely subjective: """ The recommended format is "manufacturer,model", where manufacturer is a string describing the name of the manufacturer (such as a stock ticker symbol), and model specifies the model number. "" that kind of fits in. I suppose that if we like to paint ourselves in a corner that allows us to simplify the logistics of maintaining our platforms' DTS and drivers without bending the specification we should be allowed to. How does that make your job as a DT maintainer any harder? I do not disagree that identifying the oldest SoC that featured the specific block is best, but it may not always be that simple or that descriptive either. There are a lot more properties and compatibles that are IMHO bending the Device Tree to describe how they *intend* to get the HW configured by the client program, and fall backs are really not amongst them IMHO. Anyway. > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Thanks.
From: Florian Fainelli <f.fainelli@gmail.com> On Wed, 22 Nov 2023 15:42:08 +0100, Rafał Miłecki <zajec5@gmail.com> wrote: > From: Rafał Miłecki <rafal@milecki.pl> > > It's designed for hardwiring Bluetooth devices to it. > > Signed-off-by: Rafał Miłecki <rafal@milecki.pl> > --- Applied to https://github.com/Broadcom/stblinux/commits/devicetree/next, thanks! -- Florian
diff --git a/Documentation/devicetree/bindings/serial/brcm,bcmbca-hs-uart.yaml b/Documentation/devicetree/bindings/serial/brcm,bcmbca-hs-uart.yaml new file mode 100644 index 000000000000..64ef9eee7be2 --- /dev/null +++ b/Documentation/devicetree/bindings/serial/brcm,bcmbca-hs-uart.yaml @@ -0,0 +1,60 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/serial/brcm,bcmbca-hs-uart.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Broadcom Broadband SoC High Speed UART + +description: + High speed serial port controller that was designed to handle Bluetooth + devices communication. It supports sending custom frames that need to be + processed by a host system. + +maintainers: + - Rafał Miłecki <rafal@milecki.pl> + +allOf: + - $ref: serial.yaml# + +properties: + compatible: + items: + - enum: + - brcm,bcm4908-hs-uart + - brcm,bcm4912-hs-uart + - brcm,bcm6756-hs-uart + - brcm,bcm6813-hs-uart + - brcm,bcm6846-hs-uart + - brcm,bcm6855-hs-uart + - brcm,bcm6856-hs-uart + - brcm,bcm6858-hs-uart + - brcm,bcm6878-hs-uart + - brcm,bcm47622-hs-uart + - brcm,bcm63138-hs-uart + - brcm,bcm63146-hs-uart + - brcm,bcm63158-hs-uart + - brcm,bcm63178-hs-uart + - const: brcm,bcmbca-hs-uart + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + +required: + - reg + - interrupts + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + + serial@fffec400 { + compatible = "brcm,bcm63138-hs-uart", "brcm,bcmbca-hs-uart"; + reg = <0xfffec400 0x1e0>; + interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>; + };