| Message ID | 20191116064415.159899-1-bjorn.andersson@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm64: dts: qcom: msm8998-mtp: Add alias for blsp1_uart3 | expand |
On Fri, Nov 15, 2019 at 11:44 PM Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > > The msm_serial driver uses a simple counter to determine which port to > use when no alias is defined, but there's no logic to prevent this from Which port to use for what, the default console? > not colliding with what's defined by the aliases. As a result either > none or all of the active msm_serial instances must be listed as > aliases. > > Define blsp1_uart3 as "serial1" to mitigate this problem. > > Fixes: 4cffb9f2c700 ("arm64: dts: qcom: msm8998-mtp: Enable bluetooth") > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> That driver behavior seems like a strange thing to be doing. If you clarify the question above, - Reviewed-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com> > --- > arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi > index 5f101a20a20a..e08fcb426bbf 100644 > --- a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi > +++ b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi > @@ -9,6 +9,7 @@ > / { > aliases { > serial0 = &blsp2_uart1; > + serial1 = &blsp1_uart3; > }; > > chosen { > -- > 2.23.0 >
On Sat 16 Nov 14:24 PST 2019, Jeffrey Hugo wrote: > On Fri, Nov 15, 2019 at 11:44 PM Bjorn Andersson > <bjorn.andersson@linaro.org> wrote: > > > > The msm_serial driver uses a simple counter to determine which port to > > use when no alias is defined, but there's no logic to prevent this from > > Which port to use for what, the default console? > The driver defines three (3) struct uart_ports (wrapped in struct msm_ports), see msm_uart_port[] around line 1538 in msm_serial.c This means that you can have a whooping 3 instances of msm_serial in the system and per the logic found in msm_serial_probe() the allocation follows the serial%d aliases defined and for entries without an alias a simple counter, starting at 0 is used. > > not colliding with what's defined by the aliases. As a result either > > none or all of the active msm_serial instances must be listed as > > aliases. > > > > Define blsp1_uart3 as "serial1" to mitigate this problem. > > > > Fixes: 4cffb9f2c700 ("arm64: dts: qcom: msm8998-mtp: Enable bluetooth") > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > That driver behavior seems like a strange thing to be doing. > > If you clarify the question above, - > Reviewed-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com> > Thanks, I'll respin the message to properly document this behavior. Regards, Bjorn > > --- > > arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi > > index 5f101a20a20a..e08fcb426bbf 100644 > > --- a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi > > +++ b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi > > @@ -9,6 +9,7 @@ > > / { > > aliases { > > serial0 = &blsp2_uart1; > > + serial1 = &blsp1_uart3; > > }; > > > > chosen { > > -- > > 2.23.0 > >
On Sun, Nov 17, 2019 at 7:41 PM Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > > On Sat 16 Nov 14:24 PST 2019, Jeffrey Hugo wrote: > > > On Fri, Nov 15, 2019 at 11:44 PM Bjorn Andersson > > <bjorn.andersson@linaro.org> wrote: > > > > > > The msm_serial driver uses a simple counter to determine which port to > > > use when no alias is defined, but there's no logic to prevent this from > > > > Which port to use for what, the default console? > > > > The driver defines three (3) struct uart_ports (wrapped in struct > msm_ports), see msm_uart_port[] around line 1538 in msm_serial.c > > This means that you can have a whooping 3 instances of msm_serial in the > system and per the logic found in msm_serial_probe() the allocation > follows the serial%d aliases defined and for entries without an alias a > simple counter, starting at 0 is used. Ah. Clearly no one needs more than 64k of memory, err 3 uart ports :) > > > > not colliding with what's defined by the aliases. As a result either > > > none or all of the active msm_serial instances must be listed as > > > aliases. > > > > > > Define blsp1_uart3 as "serial1" to mitigate this problem. > > > > > > Fixes: 4cffb9f2c700 ("arm64: dts: qcom: msm8998-mtp: Enable bluetooth") > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > > > That driver behavior seems like a strange thing to be doing. > > > > If you clarify the question above, - > > Reviewed-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com> > > > > Thanks, I'll respin the message to properly document this behavior. Sounds good. > > Regards, > Bjorn > > > > --- > > > arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi > > > index 5f101a20a20a..e08fcb426bbf 100644 > > > --- a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi > > > +++ b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi > > > @@ -9,6 +9,7 @@ > > > / { > > > aliases { > > > serial0 = &blsp2_uart1; > > > + serial1 = &blsp1_uart3; > > > }; > > > > > > chosen { > > > -- > > > 2.23.0 > > >
diff --git a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi index 5f101a20a20a..e08fcb426bbf 100644 --- a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi @@ -9,6 +9,7 @@ / { aliases { serial0 = &blsp2_uart1; + serial1 = &blsp1_uart3; }; chosen {
The msm_serial driver uses a simple counter to determine which port to use when no alias is defined, but there's no logic to prevent this from not colliding with what's defined by the aliases. As a result either none or all of the active msm_serial instances must be listed as aliases. Define blsp1_uart3 as "serial1" to mitigate this problem. Fixes: 4cffb9f2c700 ("arm64: dts: qcom: msm8998-mtp: Enable bluetooth") Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> --- arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi | 1 + 1 file changed, 1 insertion(+)