| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 24 Apr 2023 17:15:23 +0800
From: Chen-Yu Tsai <wenst@...omium.org>
To: Nícolas F. R. A. Prado
<nfraprado@...labora.com>
Cc: Matthias Brugger <matthias.bgg@...il.com>,
AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-mediatek@...ts.infradead.org,
linux-arm-kernel@...ts.infradead.org,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>
Subject: Re: [PATCH] arm64: dts: mediatek: mt8192-asurada-hayato: Enable Bluetooth
On Fri, Apr 21, 2023 at 10:01 PM Nícolas F. R. A. Prado
<nfraprado@...labora.com> wrote:
>
> On Fri, Apr 21, 2023 at 07:03:27PM +0800, Chen-Yu Tsai wrote:
> > Hayato's Realtek WiFi/BT module has it's Bluetooth function wired to
> > UART1.
> >
> > Add and enable the relevant device nodes for it.
> >
> > Signed-off-by: Chen-Yu Tsai <wenst@...omium.org>
> > ---
> > .../dts/mediatek/mt8192-asurada-hayato-r1.dts | 80 +++++++++++++++++++
> > 1 file changed, 80 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8192-asurada-hayato-r1.dts b/arch/arm64/boot/dts/mediatek/mt8192-asurada-hayato-r1.dts
> > index 43a823990a92..6a7d7870525b 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8192-asurada-hayato-r1.dts
> > +++ b/arch/arm64/boot/dts/mediatek/mt8192-asurada-hayato-r1.dts
> > @@ -40,9 +40,89 @@ CROS_STD_MAIN_KEYMAP
> > >;
> > };
> >
> > +&pio {
> > + bt_pins: bt-pins {
> > + bt_kill: pins-bt-kill {
>
> Drop this label and for the other pinconfigs below as they'll never be
> referenced.
Ack.
> > + pinmux = <PINMUX_GPIO144__FUNC_GPIO144>; /* BT_KILL_L */
>
> I'd also drop this and the other comments, as they're already documented in the
> gpio-line-names property.
Ack.
> > + output-low;
> > + };
> > +
> > + bt_wake: pins-bt-wake {
> > + pinmux = <PINMUX_GPIO22__FUNC_GPIO22>; /* bt to wake ap */
> > + bias-pull-up;
> > + };
> > +
> > + ap_wake_bt: pins-ap-wake-bt {
> > + pinmux = <PINMUX_GPIO168__FUNC_GPIO168>; /* AP_WAKE_BT_H */
> > + output-low;
> > + };
> > + };
> > +
> > + uart1_pins: uart1-pins {
> > + pins-rx {
> > + pinmux = <PINMUX_GPIO94__FUNC_URXD1>;
> > + input-enable;
> > + bias-pull-up;
> > + };
> > +
> > + pins-tx {
> > + pinmux = <PINMUX_GPIO95__FUNC_UTXD1>;
> > + };
> > +
> > + pins-cts {
> > + pinmux = <PINMUX_GPIO166__FUNC_UCTS1>;
> > + input-enable;
> > + };
> > +
> > + pins-rts {
> > + pinmux = <PINMUX_GPIO167__FUNC_URTS1>;
> > + output-enable;
>
> Looks like the dt-binding doesn't currently support output-enable, but the
> driver does, so please just add a patch with
>
> output-enable: true
>
> on mediatek,mt8192-pinctrl.yaml
Looking at the driver and the datasheet, there is no actual output-enable
hardware. The driver simply sets an pin direction register, which is the
same thing done when setting the GPIO direction.
On the other hand, there is actually an input enable function, but the
driver also sets the pin direction. All pins have "input enable" set
by default in hardware doesn't help.
I'll drop this for now and look into fixing the pinctrl driver.
> > + };
> > + };
> > +
> > + uart1_pins_sleep: uart1-pins-sleep {
>
> "-pins" needs to come last in the name otherwise the dt-binding will complain.
Ack.
> > + pins-rx {
> > + pinmux = <PINMUX_GPIO94__FUNC_GPIO94>;
> > + input-enable;
> > + bias-pull-up;
> > + };
> > + pins-tx {
> > + pinmux = <PINMUX_GPIO95__FUNC_UTXD1>;
> > + };
> > + pins-cts {
> > + pinmux = <PINMUX_GPIO166__FUNC_UCTS1>;
> > + input-enable;
> > + };
> > + pins-rts {
> > + pinmux = <PINMUX_GPIO167__FUNC_URTS1>;
> > + output-enable;
> > + };
> > + };
> > +};
> > +
> > &touchscreen {
> > compatible = "hid-over-i2c";
> > post-power-on-delay-ms = <10>;
> > hid-descr-addr = <0x0001>;
> > vdd-supply = <&pp3300_u>;
> > };
> > +
> > +&uart1 {
> > + status = "okay";
> > + pinctrl-names = "default", "sleep";
> > + pinctrl-0 = <&uart1_pins>;
> > + pinctrl-1 = <&uart1_pins_sleep>;
> > + /delete-property/ interrupts;
> > + interrupts-extended = <&gic GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH 0>,
> > + <&pio 94 IRQ_TYPE_EDGE_FALLING>;
> > +
> > + bluetooth: bluetooth {
>
> I'd also drop this label and only introduce it if/when needed.
Ack.
> Reviewed-by: Nícolas F. R. A. Prado <nfraprado@...labora.com>
>
> Thanks,
> Nícolas
>
> > + compatible = "realtek,rtl8822cs-bt";
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&bt_pins>;
> > +
> > + enable-gpios = <&pio 144 GPIO_ACTIVE_HIGH>;
> > + device-wake-gpios = <&pio 168 GPIO_ACTIVE_HIGH>;
> > + host-wake-gpios = <&pio 22 GPIO_ACTIVE_LOW>;
> > + };
> > +};
> > --
> > 2.40.0.634.g4ca3ef3211-goog
> >
Powered by blists - more mailing lists