[v2] ARM: dts: omap4-embt2ws: Add Bluetooth

  Since the required clock is now available, add bluetooth.

Note: Firmware (bts file) from device vendor reroutes tx for some time
during initialisation and later put it back, producing timeouts in
bluetooth initialisation but ignoring that command leads to proper
initialisation.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
Depends on: https://lore.kernel.org/linux-omap/20230916100515.1650336-6-andreas@kemnade.info/T/#u

Changes in V2:
- more standard node name, removing unneeded label

 arch/arm/boot/dts/ti/omap/omap4-epson-embt2ws.dts | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
  

On 04/10/2023 09:03, Andreas Kemnade wrote:
> Since the required clock is now available, add bluetooth.
> 
> Note: Firmware (bts file) from device vendor reroutes tx for some time
> during initialisation and later put it back, producing timeouts in
> bluetooth initialisation but ignoring that command leads to proper
> initialisation.
> 
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---
> Depends on: https://lore.kernel.org/linux-omap/20230916100515.1650336-6-andreas@kemnade.info/T/#u
> 
> Changes in V2:
> - more standard node name, removing unneeded label
> 
>  arch/arm/boot/dts/ti/omap/omap4-epson-embt2ws.dts | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/ti/omap/omap4-epson-embt2ws.dts b/arch/arm/boot/dts/ti/omap/omap4-epson-embt2ws.dts
> index 46a80eacf771..6a790124bcf5 100644
> --- a/arch/arm/boot/dts/ti/omap/omap4-epson-embt2ws.dts
> +++ b/arch/arm/boot/dts/ti/omap/omap4-epson-embt2ws.dts
> @@ -447,10 +447,12 @@ &uart2 {
>  	interrupts-extended = <&wakeupgen GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH
>  			       &omap4_pmx_core OMAP4_UART2_RX>;
>  
> -	/*
> -	 * BT + GPS in WL1283 in WG7500 requiring CLK32KAUDIO of pmic
> -	 * which does not have a driver
> -	 */
> +	bluetooth-gnss {

If you changed the bindings, please share a link. If not, this is not
currently accepted and dtbs_check should point it out.

Best regards,
Krzysztof
  

[...]
> > -	/*
> > -	 * BT + GPS in WL1283 in WG7500 requiring CLK32KAUDIO of pmic
> > -	 * which does not have a driver
> > -	 */
> > +	bluetooth-gnss {  
> 
> If you changed the bindings, please share a link. If not, this is not
> currently accepted and dtbs_check should point it out.
> 

$ make dtbs_check &>dtbs.log
$ grep -q omap4-epson-embt2ws.dtb dtbs.log  && echo yes
yes
$ grep bluetooth-gnss dtbs.log
$

So no complaints...

Regards,
Andreas
  

Hi,

On Wed, 4 Oct 2023 09:59:59 +0200
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

[...]
> > diff --git a/arch/arm/boot/dts/ti/omap/omap4-epson-embt2ws.dts b/arch/arm/boot/dts/ti/omap/omap4-epson-embt2ws.dts
> > index 46a80eacf771..6a790124bcf5 100644
> > --- a/arch/arm/boot/dts/ti/omap/omap4-epson-embt2ws.dts
> > +++ b/arch/arm/boot/dts/ti/omap/omap4-epson-embt2ws.dts
> > @@ -447,10 +447,12 @@ &uart2 {
> >  	interrupts-extended = <&wakeupgen GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH
> >  			       &omap4_pmx_core OMAP4_UART2_RX>;
> >  
> > -	/*
> > -	 * BT + GPS in WL1283 in WG7500 requiring CLK32KAUDIO of pmic
> > -	 * which does not have a driver
> > -	 */
> > +	bluetooth-gnss {  
> 
> If you changed the bindings, please share a link. If not, this is not
> currently accepted and dtbs_check should point it out.
> 

according to dtbs_check:

good:
- bluetooth-gnss
- bluettooth-gnss

bad:
- bluettooth-gnnss
- tiwi (still there in pandaboards)

Regards,
Andreas
  

On Wed, 4 Oct 2023 16:53:14 +0200
Andreas Kemnade <andreas@kemnade.info> wrote:

> Hi,
> 
> On Wed, 4 Oct 2023 09:59:59 +0200
> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> 
> [...]
> > > diff --git a/arch/arm/boot/dts/ti/omap/omap4-epson-embt2ws.dts b/arch/arm/boot/dts/ti/omap/omap4-epson-embt2ws.dts
> > > index 46a80eacf771..6a790124bcf5 100644
> > > --- a/arch/arm/boot/dts/ti/omap/omap4-epson-embt2ws.dts
> > > +++ b/arch/arm/boot/dts/ti/omap/omap4-epson-embt2ws.dts
> > > @@ -447,10 +447,12 @@ &uart2 {
> > >  	interrupts-extended = <&wakeupgen GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH
> > >  			       &omap4_pmx_core OMAP4_UART2_RX>;
> > >  
> > > -	/*
> > > -	 * BT + GPS in WL1283 in WG7500 requiring CLK32KAUDIO of pmic
> > > -	 * which does not have a driver
> > > -	 */
> > > +	bluetooth-gnss {    
> > 
> > If you changed the bindings, please share a link. If not, this is not
> > currently accepted and dtbs_check should point it out.
> >   
> 
> according to dtbs_check:
> 
> good:
> - bluetooth-gnss
> - bluettooth-gnss
> 
> bad:
> - bluettooth-gnnss
> - tiwi (still there in pandaboards)
>
The only authoritative source I find here:
 Documentation/devicetree/bindings/serial/serial.yaml:
patternProperties:
  "^bluetooth|gnss|gps|mcu$":

It allows:
 everything starting with bluetooth
 everything containing gnss
 everything containing gps
 everything ending with mcu

bluetooth-gnss starts with bluetooth, so it is allowed.
Do you have anything else forbidding bluetooth-gnss?

Regards,
Andreas

> Regards,
> Andreas
> 
>
  

On 04/10/2023 17:00, Andreas Kemnade wrote:
> On Wed, 4 Oct 2023 16:53:14 +0200
> Andreas Kemnade <andreas@kemnade.info> wrote:
> 
>> Hi,
>>
>> On Wed, 4 Oct 2023 09:59:59 +0200
>> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
>>
>> [...]
>>>> diff --git a/arch/arm/boot/dts/ti/omap/omap4-epson-embt2ws.dts b/arch/arm/boot/dts/ti/omap/omap4-epson-embt2ws.dts
>>>> index 46a80eacf771..6a790124bcf5 100644
>>>> --- a/arch/arm/boot/dts/ti/omap/omap4-epson-embt2ws.dts
>>>> +++ b/arch/arm/boot/dts/ti/omap/omap4-epson-embt2ws.dts
>>>> @@ -447,10 +447,12 @@ &uart2 {
>>>>  	interrupts-extended = <&wakeupgen GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH
>>>>  			       &omap4_pmx_core OMAP4_UART2_RX>;
>>>>  
>>>> -	/*
>>>> -	 * BT + GPS in WL1283 in WG7500 requiring CLK32KAUDIO of pmic
>>>> -	 * which does not have a driver
>>>> -	 */
>>>> +	bluetooth-gnss {    
>>>
>>> If you changed the bindings, please share a link. If not, this is not
>>> currently accepted and dtbs_check should point it out.
>>>   
>>
>> according to dtbs_check:
>>
>> good:
>> - bluetooth-gnss
>> - bluettooth-gnss
>>
>> bad:
>> - bluettooth-gnnss
>> - tiwi (still there in pandaboards)
>>
> The only authoritative source I find here:
>  Documentation/devicetree/bindings/serial/serial.yaml:
> patternProperties:
>   "^bluetooth|gnss|gps|mcu$":

Yes, it's about this one.

> 
> It allows:
>  everything starting with bluetooth
>  everything containing gnss
>  everything containing gps
>  everything ending with mcu
> 
> bluetooth-gnss starts with bluetooth, so it is allowed.
> Do you have anything else forbidding bluetooth-gnss?

Heh, the pattern is wrong. This should be originally:
"^(bluetooth|gnss|gps|mcu)$":

I can fix it and also allow your use case.

Best regards,
Krzysztof
  

On 04/10/2023 09:03, Andreas Kemnade wrote:
> Since the required clock is now available, add bluetooth.
> 
> Note: Firmware (bts file) from device vendor reroutes tx for some time
> during initialisation and later put it back, producing timeouts in
> bluetooth initialisation but ignoring that command leads to proper
> initialisation.
> 
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof
  

* Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> [231005 08:47]:
> On 04/10/2023 09:03, Andreas Kemnade wrote:
> > Since the required clock is now available, add bluetooth.
> > 
> > Note: Firmware (bts file) from device vendor reroutes tx for some time
> > during initialisation and later put it back, producing timeouts in
> > bluetooth initialisation but ignoring that command leads to proper
> > initialisation.
> > 
> > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > ---
> 
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Applying into omap-for-v6.7/dt thanks.

Tony
  

* Tony Lindgren <tony@atomide.com> [231007 07:00]:
> * Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> [231005 08:47]:
> > On 04/10/2023 09:03, Andreas Kemnade wrote:
> > > Since the required clock is now available, add bluetooth.
> > > 
> > > Note: Firmware (bts file) from device vendor reroutes tx for some time
> > > during initialisation and later put it back, producing timeouts in
> > > bluetooth initialisation but ignoring that command leads to proper
> > > initialisation.
> > > 
> > > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > > ---
> > 
> > 
> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> Applying into omap-for-v6.7/dt thanks.

Looks like this causes the following #clock-cells warning so dropping
this patch.

Regards,

Tony

arch/arm/boot/dts/ti/omap/omap4-epson-embt2ws.dts:457.17-462.4: Warning (clocks_property):
/ocp/interconnect@48000000/segment@0/target-module@6c000/serial@0/bluetooth-gnss:
Missing property '#clock-cells' in node /ocp/interconnect@48000000/segment@0/targe
  

Hi Tony,

On Sat, 7 Oct 2023 10:24:42 +0300
Tony Lindgren <tony@atomide.com> wrote:

> * Tony Lindgren <tony@atomide.com> [231007 07:00]:
> > * Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> [231005 08:47]:  
> > > On 04/10/2023 09:03, Andreas Kemnade wrote:  
> > > > Since the required clock is now available, add bluetooth.
> > > > 
> > > > Note: Firmware (bts file) from device vendor reroutes tx for some time
> > > > during initialisation and later put it back, producing timeouts in
> > > > bluetooth initialisation but ignoring that command leads to proper
> > > > initialisation.
> > > > 
> > > > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > > > ---  
> > > 
> > > 
> > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>  
> > 
> > Applying into omap-for-v6.7/dt thanks.  
> 
> Looks like this causes the following #clock-cells warning so dropping
> this patch.
> 
> Regards,
> 
> Tony
> 
> arch/arm/boot/dts/ti/omap/omap4-epson-embt2ws.dts:457462.4: Warning (clocks_property):
> /ocp/interconnect@48000000/segment@0/target-module@6c000/serial@0/bluetooth-gnss:
> Missing property '#clock-cells' in node /ocp/interconnect@48000000/segment@0/targe
> 
> 
well, it depends on the wlan dts patch. I think we are better of with having
both applied now. It is a better description of the hardware, and that is what
the devicetree is about. Having wlan randomly working without undocumented
description is IMHO worse than with a clear description.
The reason why this did not come to light earlier is because I had tested it on top
of a bunch of regression fixes for the twl (which were accepted at that point) 
and I did miss the fact that I had also a had a local hack amoung them to enable
the clock by a simple register write in _probe it twl-core.c.

I think as Stephen has little comments about the last version and just not
collected anything else than fixes for -rc, I would think it simply will be
applied. I think he just has a late start.

Regards,
Andreas
  

Hi Tony,

On Sat, 7 Oct 2023 10:24:42 +0300
Tony Lindgren <tony@atomide.com> wrote:

> * Tony Lindgren <tony@atomide.com> [231007 07:00]:
> > * Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> [231005 08:47]:  
> > > On 04/10/2023 09:03, Andreas Kemnade wrote:  
> > > > Since the required clock is now available, add bluetooth.
> > > > 
> > > > Note: Firmware (bts file) from device vendor reroutes tx for some time
> > > > during initialisation and later put it back, producing timeouts in
> > > > bluetooth initialisation but ignoring that command leads to proper
> > > > initialisation.
> > > > 
> > > > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > > > ---  
> > > 
> > > 
> > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>  
> > 
> > Applying into omap-for-v6.7/dt thanks.  
> 
> Looks like this causes the following #clock-cells warning so dropping
> this patch.
> 
> Regards,
> 
> Tony
> 
> arch/arm/boot/dts/ti/omap/omap4-epson-embt2ws.dts:457.17-462.4: Warning (clocks_property):
> /ocp/interconnect@48000000/segment@0/target-module@6c000/serial@0/bluetooth-gnss:
> Missing property '#clock-cells' in node /ocp/interconnect@48000000/segment@0/targe
>

I do not get this warning on top of omap-for-v6.8/dt. So I think the
time is really there for this patch.

Regards,
Andreas
  

* Andreas Kemnade <andreas@kemnade.info> [231130 17:36]:
> On Sat, 7 Oct 2023 10:24:42 +0300
> Tony Lindgren <tony@atomide.com> wrote:
> > arch/arm/boot/dts/ti/omap/omap4-epson-embt2ws.dts:457.17-462.4: Warning (clocks_property):
> > /ocp/interconnect@48000000/segment@0/target-module@6c000/serial@0/bluetooth-gnss:
> > Missing property '#clock-cells' in node /ocp/interconnect@48000000/segment@0/targe
> >
> 
> I do not get this warning on top of omap-for-v6.8/dt. So I think the
> time is really there for this patch.

OK thanks applying into omap-for-v6.8/dt.

Tony