linux-kernel - Re: [PATCH v1 2/3] mfd: tps65086: Make interrupt line optional

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YO1aSSSankv+cAru@google.com>
Date:   Tue, 13 Jul 2021 10:18:01 +0100
From:   Lee Jones <lee.jones@...aro.org>
To:     Emil Renner Berthing <kernel@...il.dk>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Sebastian Reichel <sre@...nel.org>,
        "Andrew F. Davis" <afd@...com>, devicetree@...r.kernel.org,
        linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 2/3] mfd: tps65086: Make interrupt line optional

On Sat, 26 Jun 2021, Emil Renner Berthing wrote:

> The BeagleV Starlight v0.9 board[1] doesn't have the IRQB line routed to
> the SoC, but it is still useful to be able to reach the PMIC over I2C

What is still useful?

The GPIO and Regulator drivers?

> for the other functionality it provides.
> 
> [1] https://github.com/beagleboard/beaglev-starlight
> 
> Signed-off-by: Emil Renner Berthing <kernel@...il.dk>
> ---
>  .../devicetree/bindings/mfd/ti,tps65086.yaml  |  3 ---
>  drivers/mfd/tps65086.c                        | 21 ++++++++++---------
>  2 files changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml b/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml
> index ba638bd10a58..4b629fcc0df9 100644
> --- a/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml
> +++ b/Documentation/devicetree/bindings/mfd/ti,tps65086.yaml
> @@ -87,9 +87,6 @@ additionalProperties: false
>  required:
>    - compatible
>    - reg
> -  - interrupts
> -  - interrupt-controller
> -  - '#interrupt-cells'

I can't say that I've been keeping up with the latest DT binding
changes, but shouldn't these lines be relocated into some kind of
optional listing?

Or are optional properties omitted from documentation?

>    - gpio-controller
>    - '#gpio-cells'
>    - regulators
> diff --git a/drivers/mfd/tps65086.c b/drivers/mfd/tps65086.c
> index 341466ef20cc..cc3478ee9a64 100644
> --- a/drivers/mfd/tps65086.c
> +++ b/drivers/mfd/tps65086.c
> @@ -100,29 +100,30 @@ static int tps65086_probe(struct i2c_client *client,
>  		 (char)((version & TPS65086_DEVICEID_OTP_MASK) >> 4) + 'A',
>  		 (version & TPS65086_DEVICEID_REV_MASK) >> 6);
>  
> -	ret = regmap_add_irq_chip(tps->regmap, tps->irq, IRQF_ONESHOT, 0,
> -				  &tps65086_irq_chip, &tps->irq_data);
> -	if (ret) {
> -		dev_err(tps->dev, "Failed to register IRQ chip\n");
> -		return ret;
> +	if (tps->irq > 0) {

Are you sure that the 0th line is not a valid IRQ?

> +		ret = regmap_add_irq_chip(tps->regmap, tps->irq, IRQF_ONESHOT, 0,
> +					  &tps65086_irq_chip, &tps->irq_data);
> +		if (ret) {
> +			dev_err(tps->dev, "Failed to register IRQ chip\n");
> +			return ret;
> +		}
>  	}
>  
>  	ret = mfd_add_devices(tps->dev, PLATFORM_DEVID_AUTO, tps65086_cells,
>  			      ARRAY_SIZE(tps65086_cells), NULL, 0,
>  			      regmap_irq_get_domain(tps->irq_data));
> -	if (ret) {
> +	if (ret && tps->irq > 0)
>  		regmap_del_irq_chip(tps->irq, tps->irq_data);
> -		return ret;
> -	}
>  
> -	return 0;
> +	return ret;
>  }
>  
>  static int tps65086_remove(struct i2c_client *client)
>  {
>  	struct tps65086 *tps = i2c_get_clientdata(client);
>  
> -	regmap_del_irq_chip(tps->irq, tps->irq_data);
> +	if (tps->irq > 0)
> +		regmap_del_irq_chip(tps->irq, tps->irq_data);
>  
>  	return 0;
>  }

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog