diff mbox series

[v4,01/11] dt-bindings: media: s5p-mfc: Add mfcv12 variant

Message ID 20231025102216.50480-2-aakarsh.jain@samsung.com
State New
Headers
Series Add MFC V12 support |

Commit Message

Aakarsh Jain Oct. 25, 2023, 10:22 a.m. UTC 
  Add Tesla FSD MFC(MFC v12) compatible.

Cc: linux-fsd@tesla.com
Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
---
 .../bindings/media/samsung,s5p-mfc.yaml          | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Comments

Krzysztof Kozlowski Oct. 25, 2023, 1 p.m. UTC | #1 
On 25/10/2023 12:22, Aakarsh Jain wrote:
> Add Tesla FSD MFC(MFC v12) compatible.
> 
> Cc: linux-fsd@tesla.com
> Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> ---

No changelog and your cover letter does not explain what happened here.
Specifically, why did you decide to ignore received tag.

>  .../bindings/media/samsung,s5p-mfc.yaml          | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml b/Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml
> index 084b44582a43..c30eb309f670 100644
> --- a/Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml
> +++ b/Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml
> @@ -24,6 +24,7 @@ properties:
>            - samsung,mfc-v7                # Exynos5420
>            - samsung,mfc-v8                # Exynos5800
>            - samsung,mfc-v10               # Exynos7880
> +          - tesla,fsd-mfc                 # Tesla FSD
>        - items:
>            - enum:
>                - samsung,exynos3250-mfc    # Exynos3250
> @@ -165,6 +166,21 @@ allOf:
>            minItems: 1
>            maxItems: 2
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - tesla,fsd-mfc
> +    then:
> +      properties:
> +        clocks:
> +          maxItems: 1
> +        clock-names:
> +          items:
> +            - const: mfc
> +        iommus: false

That's odd. How so? MFC v12 does not support IOMMU?

Best regards,
Krzysztof
Aakarsh Jain Oct. 26, 2023, 1:31 p.m. UTC | #2 
Hello Krzysztof

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: 25 October 2023 18:30
> To: Aakarsh Jain <aakarsh.jain@samsung.com>; linux-arm-
> kernel@lists.infradead.org; linux-media@vger.kernel.org; linux-
> kernel@vger.kernel.org; devicetree@vger.kernel.org
> Cc: m.szyprowski@samsung.com; andrzej.hajda@intel.com;
> mchehab@kernel.org; hverkuil-cisco@xs4all.nl;
> krzysztof.kozlowski+dt@linaro.org; dillon.minfei@gmail.com;
> david.plowman@raspberrypi.com; mark.rutland@arm.com;
> robh+dt@kernel.org; conor+dt@kernel.org; linux-samsung-
> soc@vger.kernel.org; andi@etezian.org; gost.dev@samsung.com;
> alim.akhtar@samsung.com; aswani.reddy@samsung.com;
> pankaj.dubey@samsung.com; ajaykumar.rs@samsung.com; linux-
> fsd@tesla.com
> Subject: Re: [Patch v4 01/11] dt-bindings: media: s5p-mfc: Add mfcv12
> variant
> 
> On 25/10/2023 12:22, Aakarsh Jain wrote:
> > Add Tesla FSD MFC(MFC v12) compatible.
> >
> > Cc: linux-fsd@tesla.com
> > Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> > ---
> 
> No changelog and your cover letter does not explain what happened here.
> Specifically, why did you decide to ignore received tag.
> 
Last patch series we had two different patches for schema which was one for adding MFCv12 compatible string and other for adding its HW properties.
In one of the patches you gave reviewed-by tag. Since mfc dt_schema got merged already, and this is relatively  new patch so thought of getting reviewed again.

Link to those patches:
https://patchwork.kernel.org/project/linux-media/patch/20221011122516.32135-2-aakarsh.jain@samsung.com/
https://patchwork.kernel.org/project/linux-media/patch/20221011122516.32135-3-aakarsh.jain@samsung.com/    

if you are ok, I will add your reviewed-by in next patch series.

> >  .../bindings/media/samsung,s5p-mfc.yaml          | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/media/samsung,s5p-
> mfc.yaml b/Documentation/devicetree/bindings/media/samsung,s5p-
> mfc.yaml
> > index 084b44582a43..c30eb309f670 100644
> > --- a/Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml
> > +++ b/Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml
> > @@ -24,6 +24,7 @@ properties:
> >            - samsung,mfc-v7                # Exynos5420
> >            - samsung,mfc-v8                # Exynos5800
> >            - samsung,mfc-v10               # Exynos7880
> > +          - tesla,fsd-mfc                 # Tesla FSD
> >        - items:
> >            - enum:
> >                - samsung,exynos3250-mfc    # Exynos3250
> > @@ -165,6 +166,21 @@ allOf:
> >            minItems: 1
> >            maxItems: 2
> >
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - tesla,fsd-mfc
> > +    then:
> > +      properties:
> > +        clocks:
> > +          maxItems: 1
> > +        clock-names:
> > +          items:
> > +            - const: mfc
> > +        iommus: false
> 
> That's odd. How so? MFC v12 does not support IOMMU?
> 
MFC v12 do support IOMMU. But currently it is not enabled in SW (has dependencies on some of the floating dma-mapping patches) and not tested on upstream kernel. Current patch sets intend to add support for MFCv12 using reserve memory and later patches related to enable iommu will be posted (after resolving the dependencies). So I marked iommu property as false. 
Now what is your suggestion here? Should I keep iommu as false or add memory-region as below?
Ex-
  - if:
      properties:
        compatible:
          contains:
            enum:
              - tesla,fsd-mfc
    then:
      properties:
        clocks:
          maxItems: 1
        clock-names:
          items:
            - const: mfc
        memory-region:
          maxItems: 1


> Best regards,
> Krzysztof

Thanks for review.
Krzysztof Kozlowski Nov. 1, 2023, 8:24 a.m. UTC | #3 
On 26/10/2023 15:31, Aakarsh Jain wrote:
> Hello Krzysztof
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Sent: 25 October 2023 18:30
>> To: Aakarsh Jain <aakarsh.jain@samsung.com>; linux-arm-
>> kernel@lists.infradead.org; linux-media@vger.kernel.org; linux-
>> kernel@vger.kernel.org; devicetree@vger.kernel.org
>> Cc: m.szyprowski@samsung.com; andrzej.hajda@intel.com;
>> mchehab@kernel.org; hverkuil-cisco@xs4all.nl;
>> krzysztof.kozlowski+dt@linaro.org; dillon.minfei@gmail.com;
>> david.plowman@raspberrypi.com; mark.rutland@arm.com;
>> robh+dt@kernel.org; conor+dt@kernel.org; linux-samsung-
>> soc@vger.kernel.org; andi@etezian.org; gost.dev@samsung.com;
>> alim.akhtar@samsung.com; aswani.reddy@samsung.com;
>> pankaj.dubey@samsung.com; ajaykumar.rs@samsung.com; linux-
>> fsd@tesla.com
>> Subject: Re: [Patch v4 01/11] dt-bindings: media: s5p-mfc: Add mfcv12
>> variant
>>
>> On 25/10/2023 12:22, Aakarsh Jain wrote:
>>> Add Tesla FSD MFC(MFC v12) compatible.
>>>
>>> Cc: linux-fsd@tesla.com
>>> Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
>>> ---
>>
>> No changelog and your cover letter does not explain what happened here.
>> Specifically, why did you decide to ignore received tag.
>>
> Last patch series we had two different patches for schema which was one for adding MFCv12 compatible string and other for adding its HW properties.
> In one of the patches you gave reviewed-by tag. Since mfc dt_schema got merged already, and this is relatively  new patch so thought of getting reviewed again.
> 
> Link to those patches:
> https://patchwork.kernel.org/project/linux-media/patch/20221011122516.32135-2-aakarsh.jain@samsung.com/
> https://patchwork.kernel.org/project/linux-media/patch/20221011122516.32135-3-aakarsh.jain@samsung.com/    
> 
> if you are ok, I will add your reviewed-by in next patch series.

It is okay to drop Reviewed-by tag, but this should be explicitly
mentioned in the changelog with a reason.

> 
>>>  .../bindings/media/samsung,s5p-mfc.yaml          | 16 ++++++++++++++++
>>>  1 file changed, 16 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/media/samsung,s5p-
>> mfc.yaml b/Documentation/devicetree/bindings/media/samsung,s5p-
>> mfc.yaml
>>> index 084b44582a43..c30eb309f670 100644
>>> --- a/Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml
>>> +++ b/Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml
>>> @@ -24,6 +24,7 @@ properties:
>>>            - samsung,mfc-v7                # Exynos5420
>>>            - samsung,mfc-v8                # Exynos5800
>>>            - samsung,mfc-v10               # Exynos7880
>>> +          - tesla,fsd-mfc                 # Tesla FSD
>>>        - items:
>>>            - enum:
>>>                - samsung,exynos3250-mfc    # Exynos3250
>>> @@ -165,6 +166,21 @@ allOf:
>>>            minItems: 1
>>>            maxItems: 2
>>>
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            enum:
>>> +              - tesla,fsd-mfc
>>> +    then:
>>> +      properties:
>>> +        clocks:
>>> +          maxItems: 1
>>> +        clock-names:
>>> +          items:
>>> +            - const: mfc
>>> +        iommus: false
>>
>> That's odd. How so? MFC v12 does not support IOMMU?
>>
> MFC v12 do support IOMMU. But currently it is not enabled in SW (has dependencies on some of the floating dma-mapping patches) and not tested on upstream kernel. 

Bindings describe hardware, not software.

> Current patch sets intend to add support for MFCv12 using reserve memory and later patches related to enable iommu will be posted (after resolving the dependencies). So I marked iommu property as false. 
> Now what is your suggestion here? Should I keep iommu as false or add memory-region as below?

I expect complete picture of the hardware, not something limited to
current driver, so for sure iommus must be there.

Please wrap your emails according to mailing lists rules.

Best regards,
Krzysztof
Alim Akhtar Nov. 8, 2023, 5:24 p.m. UTC | #4 
Hi Aakarsh

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: Wednesday, November 1, 2023 1:54 PM
> To: Aakarsh Jain <aakarsh.jain@samsung.com>; linux-arm-
> kernel@lists.infradead.org; linux-media@vger.kernel.org; linux-
> kernel@vger.kernel.org; devicetree@vger.kernel.org
> Cc: m.szyprowski@samsung.com; andrzej.hajda@intel.com;
> mchehab@kernel.org; hverkuil-cisco@xs4all.nl;
> krzysztof.kozlowski+dt@linaro.org; dillon.minfei@gmail.com;
> david.plowman@raspberrypi.com; mark.rutland@arm.com;
> robh+dt@kernel.org; conor+dt@kernel.org; linux-samsung-
> soc@vger.kernel.org; andi@etezian.org; gost.dev@samsung.com;
> alim.akhtar@samsung.com; aswani.reddy@samsung.com;
> pankaj.dubey@samsung.com; ajaykumar.rs@samsung.com; linux-
> fsd@tesla.com
> Subject: Re: [Patch v4 01/11] dt-bindings: media: s5p-mfc: Add mfcv12
> variant
> 
> On 26/10/2023 15:31, Aakarsh Jain wrote:
> > Hello Krzysztof
> >
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >> Sent: 25 October 2023 18:30
> >> To: Aakarsh Jain <aakarsh.jain@samsung.com>; linux-arm-
> >> kernel@lists.infradead.org; linux-media@vger.kernel.org; linux-
> >> kernel@vger.kernel.org; devicetree@vger.kernel.org
> >> Cc: m.szyprowski@samsung.com; andrzej.hajda@intel.com;
> >> mchehab@kernel.org; hverkuil-cisco@xs4all.nl;
> >> krzysztof.kozlowski+dt@linaro.org; dillon.minfei@gmail.com;
> >> david.plowman@raspberrypi.com; mark.rutland@arm.com;
> >> robh+dt@kernel.org; conor+dt@kernel.org; linux-samsung-
> >> soc@vger.kernel.org; andi@etezian.org; gost.dev@samsung.com;
> >> alim.akhtar@samsung.com; aswani.reddy@samsung.com;
> >> pankaj.dubey@samsung.com; ajaykumar.rs@samsung.com; linux-
> >> fsd@tesla.com
> >> Subject: Re: [Patch v4 01/11] dt-bindings: media: s5p-mfc: Add mfcv12
> >> variant
> >>
> >> On 25/10/2023 12:22, Aakarsh Jain wrote:
> >>> Add Tesla FSD MFC(MFC v12) compatible.
> >>>
> >>> Cc: linux-fsd@tesla.com
> >>> Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> >>> ---
> >>
> >> No changelog and your cover letter does not explain what happened
> here.
> >> Specifically, why did you decide to ignore received tag.
> >>
> > Last patch series we had two different patches for schema which was one
> for adding MFCv12 compatible string and other for adding its HW properties.
> > In one of the patches you gave reviewed-by tag. Since mfc dt_schema got
> merged already, and this is relatively  new patch so thought of getting
> reviewed again.
> >
> > Link to those patches:
> > https://patchwork.kernel.org/project/linux-
> media/patch/20221011122516.32135-2-aakarsh.jain@samsung.com/
> > https://patchwork.kernel.org/project/linux-
> media/patch/20221011122516.32135-3-aakarsh.jain@samsung.com/
> >
> > if you are ok, I will add your reviewed-by in next patch series.
> 
> It is okay to drop Reviewed-by tag, but this should be explicitly mentioned in
> the changelog with a reason.
> 
> >
> >>>  .../bindings/media/samsung,s5p-mfc.yaml          | 16
> ++++++++++++++++
> >>>  1 file changed, 16 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/media/samsung,s5p-
> >> mfc.yaml b/Documentation/devicetree/bindings/media/samsung,s5p-
> >> mfc.yaml
> >>> index 084b44582a43..c30eb309f670 100644
> >>> --- a/Documentation/devicetree/bindings/media/samsung,s5p-
> mfc.yaml
> >>> +++ b/Documentation/devicetree/bindings/media/samsung,s5p-
> mfc.yaml
> >>> @@ -24,6 +24,7 @@ properties:
> >>>            - samsung,mfc-v7                # Exynos5420
> >>>            - samsung,mfc-v8                # Exynos5800
> >>>            - samsung,mfc-v10               # Exynos7880
> >>> +          - tesla,fsd-mfc                 # Tesla FSD
> >>>        - items:
> >>>            - enum:
> >>>                - samsung,exynos3250-mfc    # Exynos3250
> >>> @@ -165,6 +166,21 @@ allOf:
> >>>            minItems: 1
> >>>            maxItems: 2
> >>>
> >>> +  - if:
> >>> +      properties:
> >>> +        compatible:
> >>> +          contains:
> >>> +            enum:
> >>> +              - tesla,fsd-mfc
> >>> +    then:
> >>> +      properties:
> >>> +        clocks:
> >>> +          maxItems: 1
> >>> +        clock-names:
> >>> +          items:
> >>> +            - const: mfc
> >>> +        iommus: false
> >>
> >> That's odd. How so? MFC v12 does not support IOMMU?
> >>
> > MFC v12 do support IOMMU. But currently it is not enabled in SW (has
> dependencies on some of the floating dma-mapping patches) and not tested
> on upstream kernel.
> 
> Bindings describe hardware, not software.
> 
> > Current patch sets intend to add support for MFCv12 using reserve
> memory and later patches related to enable iommu will be posted (after
> resolving the dependencies). So I marked iommu property as false.
> > Now what is your suggestion here? Should I keep iommu as false or add
> memory-region as below?
> 
> I expect complete picture of the hardware, not something limited to current
> driver, so for sure iommus must be there.
> 
As Krzysztof mentioned, DT binding should explain all the hardware features supported by SoC / IPs. 
Incase a feature is not enabled for some reason, that need to be handled in the dts file.

> Please wrap your emails according to mailing lists rules.
> 
> Best regards,
> Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml b/Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml
index 084b44582a43..c30eb309f670 100644
--- a/Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml
+++ b/Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml
@@ -24,6 +24,7 @@  properties:
           - samsung,mfc-v7                # Exynos5420
           - samsung,mfc-v8                # Exynos5800
           - samsung,mfc-v10               # Exynos7880
+          - tesla,fsd-mfc                 # Tesla FSD
       - items:
           - enum:
               - samsung,exynos3250-mfc    # Exynos3250
@@ -165,6 +166,21 @@  allOf:
           minItems: 1
           maxItems: 2
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - tesla,fsd-mfc
+    then:
+      properties:
+        clocks:
+          maxItems: 1
+        clock-names:
+          items:
+            - const: mfc
+        iommus: false
+
 examples:
   - |
     #include <dt-bindings/clock/exynos4.h>