On Sat, Nov 25, 2023 at 01:25:22PM +0100, Luca Weiss wrote: > 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. > Personally, I would just go with what exists already and use the existing "qcom,wcn3680" compatible as-is. No one has provided evidence that there is actually something like a WCN3680(A), without a 'B' at the end. Also, all existing users of the "qcom,wcn3680" compatible upstream actually seem to refer to WCN3680B (I'm quite sure apq8039-t2 has WCN3680B, MSM8974 isn't entirely clear but iirc you mentioned there was some schematic of a MSM8974 device which says WCN3680B as well). It would be nice to have the compatible perfectly correct and complete, but I see no point to extend it with additional information that we can't properly verify. Or do you actually need a different compatible to customize something in the driver? Thanks, Stephan
