Re: [PATCH v3 50/56] drm/omap: dsi: simplify pin config

Tomi Valkeinen
2020-11-11T12:25:16Z

On 09/11/2020 13:09, Laurent Pinchart wrote:
> Hi Tomi and Sebastian,
>
> Thank you for the patch.
>
> On Thu, Nov 05, 2020 at 02:03:27PM +0200, Tomi Valkeinen wrote:
>> From: Sebastian Reichel <sebastian.reichel@collabora.com>
>>
>> Simplify DSI pin config, which always originates from DT
>> nowadays. With the code being fully contained in the DSI
>> encoder, we can drop the public structure.
>>
>> Since the function is no longer exposed, it now directly
>> takes the private DSI data pointer. This drop a pointless
>
> s/drop/drops/
>
>> conversion and means the pins can be configured earlier.
>>
>> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> ---
>> drivers/gpu/drm/omapdrm/dss/dsi.c | 33 +++++++++------------------
>> drivers/gpu/drm/omapdrm/dss/omapdss.h | 15 ------------
>> 2 files changed, 11 insertions(+), 37 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c
>> index f47d7e3bb631..76e4f607d8cf 100644
>> --- a/drivers/gpu/drm/omapdrm/dss/dsi.c
>> +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
>> @@ -3568,12 +3568,9 @@ static void dsi_proto_timings(struct dsi_data *dsi)
>> }
>> }
>>
>> -static int dsi_configure_pins(struct omap_dss_device *dssdev,
>> - const struct omap_dsi_pin_config *pin_cfg)
>> +static int dsi_configure_pins(struct dsi_data *dsi,
>> + int num_pins, const u32 *pins)
>> {
>> - struct dsi_data *dsi = to_dsi_data(dssdev);
>> - int num_pins;
>> - const int *pins;
>> struct dsi_lane_config lanes[DSI_MAX_NR_LANES];
>> int num_lanes;
>> int i;
>> @@ -3586,9 +3583,6 @@ static int dsi_configure_pins(struct omap_dss_device *dssdev,
>> DSI_LANE_DATA4,
>> };
>>
>> - num_pins = pin_cfg->num_pins;
>> - pins = pin_cfg->pins;
>> -
>> if (num_pins < 4 || num_pins > dsi->num_lanes_supported * 2
>> || num_pins % 2 != 0)
>> return -EINVAL;
>> @@ -3600,7 +3594,7 @@ static int dsi_configure_pins(struct omap_dss_device *dssdev,
>>
>> for (i = 0; i < num_pins; i += 2) {
>> u8 lane, pol;
>> - int dx, dy;
>> + u32 dx, dy;
>
> Is this change needed ?

The pins array is now u32, so the above is correct. However, a bit below we check if dx < 0, which
doesn't make sense anymore, so I'll drop that check.

Tomi