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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
> Date:
> 01/07/24, 3:33 pm
> 
> To:
> Manikandan Muralidharan <manikandan.m@xxxxxxxxxxxxx>
> CC:
> megi@xxxxxx, javierm@xxxxxxxxxx, neil.armstrong@xxxxxxxxxx, 
> quic_jesszhan@xxxxxxxxxxx, sam@xxxxxxxxxxxx, airlied@xxxxxxxxx, 
> daniel@xxxxxxxx, maarten.lankhorst@xxxxxxxxxxxxxxx, mripard@xxxxxxxxxx, 
> tzimmermann@xxxxxxx, robh@xxxxxxxxxx, krzk+dt@xxxxxxxxxx, 
> conor+dt@xxxxxxxxxx, dri-devel@xxxxxxxxxxxxxxxxxxxxx, 
> devicetree@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx
> 
> 
> 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@xxxxxxxxxxxxx>
>> ---
>> 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.





[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux