Re: [PATCH v10 4/8] drm/ingenic: Add dw-hdmi driver for jz4780

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

 



Hi Nikolaus, Mark,

Le mer., déc. 1 2021 at 14:51:51 +0100, H. Nikolaus Schaller <hns@xxxxxxxxxxxxx> a écrit :
Hi,

 Am 01.12.2021 um 14:39 schrieb Mark Brown <broonie@xxxxxxxxxx>:

 On Wed, Dec 01, 2021 at 01:02:45PM +0000, Paul Cercueil wrote:
 Le mar., nov. 30 2021 at 22:26:37 +0100, H. Nikolaus Schaller

 +	regulator = devm_regulator_get_optional(&pdev->dev, "hdmi-5v");
 +	if (IS_ERR(regulator)) {
 +		ret = PTR_ERR(regulator);

Why is this using _optional()? This should only be done when the supply
 can be physically absent

There can be +5V for HDMI but without a regulator that is visible to or controllable
by the driver.

There is always a power supply though. Either a controllable one (through e.g. a GPIO), or it's just connected to the mains +5V; the pin is never left floating. In the second case, in DTS the "hdmi-5v" would be connected to some 5v regulator, even if it's just a dummy VCC-5V regulator. So Mark has a point.

So hdmi-5v can be simply missing in DTS in which case the driver does not need to
care about. The driver just can't turn it on or off.

Please make it mandatory in DTS then, and use devm_regulator_get() in the driver.

Cheers,
-Paul

 (in which case I'd expect to see special
 handling).

The special case is to not enable/disable the regulator if it does not exist and assume that there is hardware providing it otherwise (the driver can't know
that except by using get_optional). This is done by the code below

 +	if (IS_ERR(regulator)) {

...

 +	if (!regulator)
 +		return 0;

 +	ret = regulator_enable(regulator);

...

BR and thanks,
Nikolaus










[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