[v2,2/2] arm64: dts: mediatek: mt8183-pico6: Fix bluetooth node

Message ID 20240126063500.2684087-3-wenst@chromium.org
State New
Headers
Series bluetooth: mt7921s: Add binding and fixup existing dts |

Commit Message

Chen-Yu Tsai Jan. 26, 2024, 6:34 a.m. UTC
  Bluetooth is not a random device connected to the MMC/SD controller. It
is function 2 of the SDIO device.

Fix the address of the bluetooth node. Also fix the node name and drop
the label.

Fixes: 055ef10ccdd4 ("arm64: dts: mt8183: Add jacuzzi pico/pico6 board")
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
Changes since v1:
- Collected reviewed-by

 arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-pico6.dts | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
  

Comments

Matthias Brugger Jan. 29, 2024, 3:51 p.m. UTC | #1
On 26/01/2024 07:34, Chen-Yu Tsai wrote:
> Bluetooth is not a random device connected to the MMC/SD controller. It
> is function 2 of the SDIO device.
> 
> Fix the address of the bluetooth node. Also fix the node name and drop
> the label.
> 
> Fixes: 055ef10ccdd4 ("arm64: dts: mt8183: Add jacuzzi pico/pico6 board")
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>

> ---
> Changes since v1:
> - Collected reviewed-by
> 
>   arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-pico6.dts | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-pico6.dts b/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-pico6.dts
> index a2e74b829320..6a7ae616512d 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-pico6.dts
> +++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-pico6.dts
> @@ -82,7 +82,8 @@ pins-clk {
>   };
>   
>   &mmc1 {
> -	bt_reset: bt-reset {
> +	bluetooth@2 {
> +		reg = <2>;
>   		compatible = "mediatek,mt7921s-bluetooth";
>   		pinctrl-names = "default";
>   		pinctrl-0 = <&bt_pins_reset>;
  
Paul Menzel Jan. 29, 2024, 4:31 p.m. UTC | #2
Dear Chen-Yu,


Thank you for your patch.

Am 26.01.24 um 07:34 schrieb Chen-Yu Tsai:
> Bluetooth is not a random device connected to the MMC/SD controller. It
> is function 2 of the SDIO device.
> 
> Fix the address of the bluetooth node. Also fix the node name and drop
> the label.

Excuse my ignorance: Is this a cosmetic fix or does it fix the device 
somehow?

> Fixes: 055ef10ccdd4 ("arm64: dts: mt8183: Add jacuzzi pico/pico6 board")
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
> Changes since v1:
> - Collected reviewed-by
> 
>   arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-pico6.dts | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-pico6.dts b/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-pico6.dts
> index a2e74b829320..6a7ae616512d 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-pico6.dts
> +++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-pico6.dts
> @@ -82,7 +82,8 @@ pins-clk {
>   };
>   
>   &mmc1 {
> -	bt_reset: bt-reset {
> +	bluetooth@2 {
> +		reg = <2>;

To avoid confusion, would it be possible to use sdio as a “name”.

>   		compatible = "mediatek,mt7921s-bluetooth";
>   		pinctrl-names = "default";
>   		pinctrl-0 = <&bt_pins_reset>;


Kind regards,

Paul
  
AngeloGioacchino Del Regno Jan. 29, 2024, 4:36 p.m. UTC | #3
Il 29/01/24 17:31, Paul Menzel ha scritto:
> Dear Chen-Yu,
> 
> 
> Thank you for your patch.
> 
> Am 26.01.24 um 07:34 schrieb Chen-Yu Tsai:
>> Bluetooth is not a random device connected to the MMC/SD controller. It
>> is function 2 of the SDIO device.
>>
>> Fix the address of the bluetooth node. Also fix the node name and drop
>> the label.
> 
> Excuse my ignorance: Is this a cosmetic fix or does it fix the device somehow?
> 
>> Fixes: 055ef10ccdd4 ("arm64: dts: mt8183: Add jacuzzi pico/pico6 board")
>> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
>> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> ---
>> Changes since v1:
>> - Collected reviewed-by
>>
>>   arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-pico6.dts | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-pico6.dts 
>> b/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-pico6.dts
>> index a2e74b829320..6a7ae616512d 100644
>> --- a/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-pico6.dts
>> +++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-pico6.dts
>> @@ -82,7 +82,8 @@ pins-clk {
>>   };
>>   &mmc1 {
>> -    bt_reset: bt-reset {
>> +    bluetooth@2 {
>> +        reg = <2>;
> 
> To avoid confusion, would it be possible to use sdio as a “name”.
> 

Names must be generic and descriptive; this is the Bluetooth part of the MT7921s
hence it's called "bluetooth". If its functionality was "sdio", it'd be as such,
but this is BT anyway.

"sdio@xxx" would be applicable to, for example, a controller that provides support
exclusively for SDIO (with no support for eMMC/SD).

Regards,
Angelo
  
Chen-Yu Tsai Jan. 30, 2024, 3:21 a.m. UTC | #4
On Tue, Jan 30, 2024 at 12:31 AM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>
> Dear Chen-Yu,
>
>
> Thank you for your patch.
>
> Am 26.01.24 um 07:34 schrieb Chen-Yu Tsai:
> > Bluetooth is not a random device connected to the MMC/SD controller. It
> > is function 2 of the SDIO device.
> >
> > Fix the address of the bluetooth node. Also fix the node name and drop
> > the label.
>
> Excuse my ignorance: Is this a cosmetic fix or does it fix the device
> somehow?

It's a cosmetic change, since the driver already searches the whole device
tree for the specific compatible string. However it also fixes the device
tree description to match the actual hardware.

> > Fixes: 055ef10ccdd4 ("arm64: dts: mt8183: Add jacuzzi pico/pico6 board")
> > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> > Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> > ---
> > Changes since v1:
> > - Collected reviewed-by
> >
> >   arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-pico6.dts | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-pico6.dts b/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-pico6.dts
> > index a2e74b829320..6a7ae616512d 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-pico6.dts
> > +++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-pico6.dts
> > @@ -82,7 +82,8 @@ pins-clk {
> >   };
> >
> >   &mmc1 {
> > -     bt_reset: bt-reset {
> > +     bluetooth@2 {
> > +             reg = <2>;
>
> To avoid confusion, would it be possible to use sdio as a “name”.

Not sure where the confusion is. Bluetooth is the functionality this
SDIO function provides.

ChenYu

>
> >               compatible = "mediatek,mt7921s-bluetooth";
> >               pinctrl-names = "default";
> >               pinctrl-0 = <&bt_pins_reset>;
>
>
> Kind regards,
>
> Paul
  

Patch

diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-pico6.dts b/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-pico6.dts
index a2e74b829320..6a7ae616512d 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-pico6.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-pico6.dts
@@ -82,7 +82,8 @@  pins-clk {
 };
 
 &mmc1 {
-	bt_reset: bt-reset {
+	bluetooth@2 {
+		reg = <2>;
 		compatible = "mediatek,mt7921s-bluetooth";
 		pinctrl-names = "default";
 		pinctrl-0 = <&bt_pins_reset>;