On Montag, 16. Oktober 2023 16:44:28 CET Stephan Gerhold wrote: > On Mon, Oct 16, 2023 at 03:16:14PM +0200, Loic Poulain wrote: > > On Mon, 16 Oct 2023 at 07:35, Krzysztof Kozlowski > > > > <krzysztof.kozlowski@xxxxxxxxxx> wrote: > > > On 15/10/2023 22:03, Luca Weiss wrote: > > > > Add a compatible for the iris subnode in the WCNSS PIL. > > > > > > > > Signed-off-by: Luca Weiss <luca@xxxxxxxxx> > > > > --- > > > > > > > > Documentation/devicetree/bindings/remoteproc/qcom,wcnss-pil.yaml | 1 > > > > + > > > > 1 file changed, 1 insertion(+) > > > > > > > > diff --git > > > > a/Documentation/devicetree/bindings/remoteproc/qcom,wcnss-pil.yaml > > > > b/Documentation/devicetree/bindings/remoteproc/qcom,wcnss-pil.yaml > > > > index 45eb42bd3c2c..0e5e0b7a0610 100644 > > > > --- a/Documentation/devicetree/bindings/remoteproc/qcom,wcnss-pil.yaml > > > > +++ b/Documentation/devicetree/bindings/remoteproc/qcom,wcnss-pil.yaml > > > > > > > > @@ -111,6 +111,7 @@ properties: > > > > - qcom,wcn3660 > > > > - qcom,wcn3660b > > > > - qcom,wcn3680 > > > > > > > > + - qcom,wcn3680b > > > > > > Looks like this should be made as compatible with qcom,wcn3680 (so with > > > fallback). > > > > Yes, agree, let's do a regular fallback as there is nothing 'b' > > specific in the driver: > > `compatible = "qcom,wcn3680b", "qcom,wcn3680";` > > > > And yes, we should also have done that for qcom,wcn3660b... > > I don't think this would have worked properly for qcom,wcn3660b: > > - It's not compatible with "qcom,wcn3660", because they have different > regulator voltage requirements. wcn3660(a?) needs vddpa with > 2.9-3.0V, but wcn3660b needs 3.3V. That's why wcn3660b uses the > wcn3680_data in qcom_wcnss.iris.c. Otherwise if you would run an > older kernel that knows "qcom,wcn3660" but not "qcom,wcn3660b" it > would apply the wrong voltage. > > - It's not compatible with "qcom,wcn3680" either because that is used > as indication if 802.11ac is supported (wcn3660b doesn't). > > The main question here is: What does the current "qcom,wcn3680" > compatible actually represent? It's defined with vddpa = 3.3V in the > driver, which would suggest that: > > 1. It's actually meant to represent WCN3680B, which needs 3.3V vddpa > like WCN3660B, or > > 2. WCN3680(A?) has different requirements than WCN3660(A?) and also > needs 3.3V vddpa. But then what is the difference between > WCN3680(A?) and WCN3680B? Is there even a variant without ...B? > > There is public documentation for WCN3660B and WCN3680B but the non-B > variants are shrouded in mystery. Hi Stephan (and everyone), Do you have a suggestion how to move this patchset forward? Is the fallback compatible that was suggested okay for the wcn3680b situation? compatible = "qcom,wcn3680b", "qcom,wcn3680"; If so, I'll make v2 with that implemented. Regards Luca > > Thanks, > Stephan