Re: [PATCH v2 1/3] dt-bindings: display: himax-hx8394: Add Microchip AC40T08A MIPI Display panel

[<Manikandan.M@microchip.com>]

Hi Conor,

On 01/07/24 3:33 pm, Conor Dooley wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> 
> ForwardedMessage.eml
> 
> Subject:
> Re: [PATCH v2 1/3] dt-bindings: display: himax-hx8394: Add Microchip 
> AC40T08A MIPI Display panel
> From:
> Conor Dooley <conor@kernel.org>
> Date:
> 01/07/24, 3:33 pm
> 
> To:
> Manikandan Muralidharan <manikandan.m@microchip.com>
> CC:
> megi@xff.cz, javierm@redhat.com, neil.armstrong@linaro.org, 
> quic_jesszhan@quicinc.com, sam@ravnborg.org, airlied@gmail.com, 
> daniel@ffwll.ch, maarten.lankhorst@linux.intel.com, mripard@kernel.org, 
> tzimmermann@suse.de, robh@kernel.org, krzk+dt@kernel.org, 
> conor+dt@kernel.org, dri-devel@lists.freedesktop.org, 
> devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
> 
> 
> On Mon, Jul 01, 2024 at 02:28:35PM +0530, Manikandan Muralidharan wrote:
>> Add compatible string for the Microchip's AC40T08A MIPI Display
>> panel.This panel uses a Himax HX8394 display controller.
>> The reset line is not populated and leads to driver probe issues,
>> thus add conditional block to narrow reset-gpio property per variant.
> I really should have asked on v1, but I wasn't sure whether or not the
> optional nature of the reset-gpios was specific to your new panel so I
> held off: Is it ever the case that a reset-gpio can be provided for this
> microchip panel, or just not in the configuration you tested? If it is
> never possible, then I'd probably do...
The reset-gpio line can be provided for the microchip MIPI panel, the 
SoC variant in which I tested did not have the reset line configured, 
hence had to make it optional here and also in the driver to avoid panel 
probe issues.
> 
>> Signed-off-by: Manikandan Muralidharan<manikandan.m@microchip.com>
>> ---
>> changes in v2:
>> - re-order compatible string alphabetically.
>> - Add conditional block to narrow reset-gpio property from required
>> list based on compatible string check
>> ---
>>   .../bindings/display/panel/himax,hx8394.yaml    | 17 +++++++++++++----
>>   1 file changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/display/panel/himax,hx8394.yaml b/Documentation/devicetree/bindings/display/panel/himax,hx8394.yaml
>> index 644387e4fb6f..75ccabff308b 100644
>> --- a/Documentation/devicetree/bindings/display/panel/himax,hx8394.yaml
>> +++ b/Documentation/devicetree/bindings/display/panel/himax,hx8394.yaml
>> @@ -15,14 +15,12 @@ description:
>>     such as the HannStar HSD060BHW4 720x1440 TFT LCD panel connected with
>>     a MIPI-DSI video interface.
>>   
>> -allOf:
>> -  - $ref: panel-common.yaml#
>> -
>>   properties:
>>     compatible:
>>       items:
>>         - enum:
>>             - hannstar,hsd060bhw4
>> +          - microchip,ac40t08a-mipi-panel
>>             - powkiddy,x55-panel
>>         - const: himax,hx8394
>>   
>> @@ -46,7 +44,6 @@ properties:
>>   required:
>>     - compatible
>>     - reg
>> -  - reset-gpios
>>     - backlight
>>     - port
>>     - vcc-supply
>> @@ -54,6 +51,18 @@ required:
>>   
>>   additionalProperties: false
>>   
>> +allOf:
>> +  - $ref: panel-common.yaml#
>> +  - if:
>> +      not:
>> +        properties:
>> +          compatible:
>> +            enum:
>> +              - microchip,ac40t08a-mipi-panel
>> +    then:
>> +      required:
>> +        - reset-gpios
>    - if:
>        properties:
>          compatible:
>            const: microchip,ac40t08a-mipi-panel
>      then:
>        properties:
>          reset-gpios: false
>      else:
>        required:
>          - reset-gpios
> 
> Otherwise, what you have is fine.
> 
> Cheers,
> Conor.
> 
>>   examples:
>>     - |
>>       #include <dt-bindings/gpio/gpio.h>
>> -- 
>> 2.25.1
>>

-- 
Thanks and Regards,
Manikandan M.