Re: [PATCH 3/5] gpu: drm: adp: Add a backlight driver for the Summit LCD

[Krzysztof Kozlowski <krzk@kernel.org>]

On 25/11/2024 16:03, Nick Chan wrote:
>>> +static int summit_probe(struct mipi_dsi_device *dsi)
>>> +{
>>> +	struct backlight_properties props = { 0 };
>>> +	struct device *dev = &dsi->dev;
>>> +	struct summit_data *panel;
>>> +
>>> +	panel = devm_kzalloc(dev, sizeof(*panel), GFP_KERNEL);
>>> +	if (!panel)
>>> +		return -ENOMEM;
>>> +
>>> +	mipi_dsi_set_drvdata(dsi, panel);
>>> +	panel->dsi = dsi;
>>> +
>>> +	int ret = device_property_read_u32(dev, "max-brightness", &props.max_brightness);
>> That's an undocumented property, which suggests you did not test your DTS.
> 
> Actually, testing the DTS would not have caught this issue. For more
> context,

If you mean that property is not in your DTS, then right, it would not
be caught. But otherwise it would.

Anyway, I pointed out testing as helping tool for your development, just
like building with clang W=1 will spare you some review comments or bugs.

> all summit panels found in touch bar have a max brightness of 255, but the
> summit panel in Apple A11 devices like the iPhone X is latter found to have
> a max brightness of 2047.
> 
> However, A11 cannot be properly supported right now due to not having a
> driver
> for the DART IOMMU.
> 
> In the meantime, max-brightness could documented and be made required,
> and the
> default 255 brightness could be removed.

BTW, max-brightness is a property of backlight, not panel, I think.

Best regards,
Krzysztof