[PATCH 0/2] omap: hsi: Fix warnings on boards that don't use ssi ports

[PATCH 0/2] omap: hsi: Fix warnings on boards that don't use ssi ports

[Roger Quadros]

Hi,

This series fixes warnings and console noise for boards that don't use
ssi ports. Tested on beagleboard-c4.

cheers,
-roger

Roger Quadros (2):
  hsi: omap_ssi_port: Prevent warning if cawake_gpio is not defined.
  ARM: dts: omap3: keep ssi ports by default

 arch/arm/boot/dts/omap3-n900.dts        | 1 +
 arch/arm/boot/dts/omap3.dtsi            | 2 ++
 drivers/hsi/controllers/omap_ssi_port.c | 2 +-
 3 files changed, 4 insertions(+), 1 deletion(-)

-- 
2.1.4

[PATCH 1/2] hsi: omap_ssi_port: Prevent warning if cawake_gpio is not defined.

[Roger Quadros]

The error handling path is broken as cawake_gpio was defined as
unsigned integer causing the following warnings on boards that don't
use SSI port and so don't have cawake_gpio defined. e.g. beagleboard C4.

[   30.094635] WARNING: CPU: 0 PID: 322 at drivers/gpio/gpiolib.c:86 gpio_to_desc+0xa4/0xb8()
[   30.103363] invalid GPIO -2
[   30.106292] Modules linked in: omap_ssi_port(+) cpufreq_dt cfbfillrect cfbimgblt leds_gpio cfbcopyarea thermal_sys led_class hwmon gpio_keys encoder_tfp410 connector_analog_tv connector_dvi omap_hdq snd phy_i
[   30.145477] CPU: 0 PID: 322 Comm: modprobe Not tainted 4.3.0-rc4-00030-gca978c0-dirty #335
[   30.154174] Hardware name: Generic OMAP3-GP (Flattened Device Tree)
[   30.160827] [<c0016ef4>] (unwind_backtrace) from [<c00131f4>] (show_stack+0x10/0x14)
[   30.168975] [<c00131f4>] (show_stack) from [<c033cf08>] (dump_stack+0x80/0x9c)
[   30.176635] [<c033cf08>] (dump_stack) from [<c003e920>] (warn_slowpath_common+0x7c/0xb8)
[   30.185180] [<c003e920>] (warn_slowpath_common) from [<c003e9f0>] (warn_slowpath_fmt+0x30/0x40)
[   30.194366] [<c003e9f0>] (warn_slowpath_fmt) from [<c0376314>] (gpio_to_desc+0xa4/0xb8)
[   30.202819] [<c0376314>] (gpio_to_desc) from [<c0376ac8>] (gpio_request_one+0x14/0x11c)
[   30.211273] [<c0376ac8>] (gpio_request_one) from [<c037370c>] (devm_gpio_request_one+0x3c/0x78)
[   30.220458] [<c037370c>] (devm_gpio_request_one) from [<bf184210>] (ssi_port_probe+0x118/0x504 [omap_ssi_port])
[   30.231170] [<bf184210>] (ssi_port_probe [omap_ssi_port]) from [<c03d4cfc>] (platform_drv_probe+0x48/0xa4)
[   30.241424] [<c03d4cfc>] (platform_drv_probe) from [<c03d3678>] (driver_probe_device+0x1dc/0x2a0)
[   30.250793] [<c03d3678>] (driver_probe_device) from [<c03d37d0>] (__driver_attach+0x94/0x98)
[   30.259643] [<c03d37d0>] (__driver_attach) from [<c03d1d60>] (bus_for_each_dev+0x54/0x88)
[   30.268249] [<c03d1d60>] (bus_for_each_dev) from [<c03d2d50>] (bus_add_driver+0xe8/0x1f8)
[   30.276916] [<c03d2d50>] (bus_add_driver) from [<c03d4118>] (driver_register+0x78/0xf4)
[   30.285369] [<c03d4118>] (driver_register) from [<c03d5380>] (__platform_driver_probe+0x34/0xd8)
[   30.294647] [<c03d5380>] (__platform_driver_probe) from [<c00097e4>] (do_one_initcall+0x80/0x1d8)
[   30.303985] [<c00097e4>] (do_one_initcall) from [<c011617c>] (do_init_module+0x5c/0x1cc)
[   30.312561] [<c011617c>] (do_init_module) from [<c00c7a68>] (load_module+0x18c8/0x1f0c)
[   30.320983] [<c00c7a68>] (load_module) from [<c00c8188>] (SyS_init_module+0xdc/0x150)
[   30.329223] [<c00c8188>] (SyS_init_module) from [<c000f7e0>] (ret_fast_syscall+0x0/0x1c)

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/hsi/controllers/omap_ssi_port.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hsi/controllers/omap_ssi_port.c b/drivers/hsi/controllers/omap_ssi_port.c
index 1f8652b..02e6603 100644
--- a/drivers/hsi/controllers/omap_ssi_port.c
+++ b/drivers/hsi/controllers/omap_ssi_port.c
@@ -1111,7 +1111,7 @@ static int __init ssi_port_probe(struct platform_device *pd)
 	struct omap_ssi_port *omap_port;
 	struct hsi_controller *ssi = dev_get_drvdata(pd->dev.parent);
 	struct omap_ssi_controller *omap_ssi = hsi_controller_drvdata(ssi);
-	u32 cawake_gpio = 0;
+	int cawake_gpio = 0;
 	u32 port_id;
 	int err;
 
-- 
2.1.4

Re: [PATCH 1/2] hsi: omap_ssi_port: Prevent warning if cawake_gpio is not defined.

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 404 bytes --]

Hi,

On Wed, Oct 14, 2015 at 01:44:15PM +0300, Roger Quadros wrote:
> The error handling path is broken as cawake_gpio was defined as
> unsigned integer causing the following warnings on boards that don't
> use SSI port and so don't have cawake_gpio defined. e.g. beagleboard C4.
>
> [...]

Thanks, I queued this one together with a

Fixes: b209e047bc743 ("HSI: Introduce OMAP SSI driver")

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH 2/2] ARM: dts: omap3: keep ssi ports by default

[Roger Quadros]

Let's keep the SSI ports disabled in the omap3.dtsi to avoid
getting the following noise on the console for boards that don't
use the SSI ports.

"omap_ssi_port 4805a000.ssi-port: DT data is missing cawake gpio (err=-2)"

As omap3-n900 uses one SSI port, mark it enabled there.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 arch/arm/boot/dts/omap3-n900.dts | 1 +
 arch/arm/boot/dts/omap3.dtsi     | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
index 5f5e0f3..bdb72fb 100644
--- a/arch/arm/boot/dts/omap3-n900.dts
+++ b/arch/arm/boot/dts/omap3-n900.dts
@@ -949,6 +949,7 @@
 };
 
 &ssi_port1 {
+	status = "ok";
 	pinctrl-names = "default";
 	pinctrl-0 = <&ssi_pins>;
 
diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
index 8a2b253..25b0b37 100644
--- a/arch/arm/boot/dts/omap3.dtsi
+++ b/arch/arm/boot/dts/omap3.dtsi
@@ -815,6 +815,7 @@
 				interrupt-parent = <&intc>;
 				interrupts = <67>,
 					     <68>;
+				status = "disabled";
 			};
 
 			ssi_port2: ssi-port@4805b000 {
@@ -828,6 +829,7 @@
 				interrupt-parent = <&intc>;
 				interrupts = <69>,
 					     <70>;
+				status = "disabled";
 			};
 		};
 	};
-- 
2.1.4

Re: [PATCH 2/2] ARM: dts: omap3: keep ssi ports by default

[Pali Rohár]

On Wednesday 14 October 2015 13:44:16 Roger Quadros wrote:
> Let's keep the SSI ports disabled in the omap3.dtsi to avoid
> getting the following noise on the console for boards that don't
> use the SSI ports.
> 
> "omap_ssi_port 4805a000.ssi-port: DT data is missing cawake gpio (err=-2)"
> 
> As omap3-n900 uses one SSI port, mark it enabled there.
> 

Sebastian, why is second SSI port disabled on N900? It is not used or
in DTS file are missing some definitions?

-- 
Pali Rohár
pali.rohar@gmail.com

Re: [PATCH 2/2] ARM: dts: omap3: keep ssi ports by default

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1687 bytes --]

Hi Pali,

On Wed, Oct 14, 2015 at 12:53:04PM +0200, Pali Rohár wrote:
> On Wednesday 14 October 2015 13:44:16 Roger Quadros wrote:
> > Let's keep the SSI ports disabled in the omap3.dtsi to avoid
> > getting the following noise on the console for boards that don't
> > use the SSI ports.
> > 
> > "omap_ssi_port 4805a000.ssi-port: DT data is missing cawake gpio (err=-2)"
> > 
> > As omap3-n900 uses one SSI port, mark it enabled there.
> > 
> 
> Sebastian, why is second SSI port disabled on N900? It is not used or
> in DTS file are missing some definitions?

Since the second SSI port is not used on the Nokia N900. It's pins
are used for other things, e.g. as GPIO. It's easily verifiable
by checking the mux data:

$ grep -C1 "ssi2_" arch/arm/mach-omap2/mux34xx.c
	_OMAP3_MUXENTRY(CAM_D10, 109,
		"cam_d10", "ssi2_wake", NULL, NULL,
		"gpio_109", "hw_dbg8", NULL, "safe_mode"),
	_OMAP3_MUXENTRY(CAM_D2, 101,
		"cam_d2", "ssi2_rdy_tx", NULL, NULL,
		"gpio_101", "hw_dbg4", NULL, "safe_mode"),
	_OMAP3_MUXENTRY(CAM_D3, 102,
		"cam_d3", "ssi2_dat_rx", NULL, NULL,
		"gpio_102", "hw_dbg5", NULL, "safe_mode"),
	_OMAP3_MUXENTRY(CAM_D4, 103,
		"cam_d4", "ssi2_flag_rx", NULL, NULL,
		"gpio_103", "hw_dbg6", NULL, "safe_mode"),
	_OMAP3_MUXENTRY(CAM_D5, 104,
		"cam_d5", "ssi2_rdy_rx", NULL, NULL,
		"gpio_104", "hw_dbg7", NULL, "safe_mode"),
	_OMAP3_MUXENTRY(CAM_HS, 94,
		"cam_hs", "ssi2_dat_tx", NULL, NULL,
		"gpio_94", "hw_dbg0", NULL, "safe_mode"),
	_OMAP3_MUXENTRY(CAM_VS, 95,
		"cam_vs", "ssi2_flag_tx", NULL, NULL,
		"gpio_95", "hw_dbg1", NULL, "safe_mode"),

At least gpio 104 is already used for resetting the tsc2005.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 2/2] ARM: dts: omap3: keep ssi ports by default

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 486 bytes --]

Hi,

On Wed, Oct 14, 2015 at 01:44:16PM +0300, Roger Quadros wrote:
> Let's keep the SSI ports disabled in the omap3.dtsi to avoid
> getting the following noise on the console for boards that don't
> use the SSI ports.
> 
> "omap_ssi_port 4805a000.ssi-port: DT data is missing cawake gpio (err=-2)"
> 
> As omap3-n900 uses one SSI port, mark it enabled there.
> 
> Signed-off-by: Roger Quadros <rogerq@ti.com>

Acked-By: Sebastian Reichel <sre@kernel.org>

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 2/2] ARM: dts: omap3: keep ssi ports by default

[Roger Quadros]

[-- Attachment #1: Type: text/plain, Size: 646 bytes --]

On 14/10/15 14:19, Sebastian Reichel wrote:
> Hi,
> 
> On Wed, Oct 14, 2015 at 01:44:16PM +0300, Roger Quadros wrote:
>> Let's keep the SSI ports disabled in the omap3.dtsi to avoid
>> getting the following noise on the console for boards that don't
>> use the SSI ports.
>>
>> "omap_ssi_port 4805a000.ssi-port: DT data is missing cawake gpio (err=-2)"
>>
>> As omap3-n900 uses one SSI port, mark it enabled there.
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
> 
> Acked-By: Sebastian Reichel <sre@kernel.org>

Would it be preferable to disable the ssi-controller node as well in the
omap3.dtsi file?

cheers,
-roger


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 2/2] ARM: dts: omap3: keep ssi ports by default

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 887 bytes --]

Hi,

On Wed, Oct 14, 2015 at 02:27:27PM +0300, Roger Quadros wrote:
> On 14/10/15 14:19, Sebastian Reichel wrote:
> > On Wed, Oct 14, 2015 at 01:44:16PM +0300, Roger Quadros wrote:
> >> Let's keep the SSI ports disabled in the omap3.dtsi to avoid
> >> getting the following noise on the console for boards that don't
> >> use the SSI ports.
> >>
> >> "omap_ssi_port 4805a000.ssi-port: DT data is missing cawake gpio (err=-2)"
> >>
> >> As omap3-n900 uses one SSI port, mark it enabled there.
>
> Would it be preferable to disable the ssi-controller node as well in the
> omap3.dtsi file?

If I remember it correctly, existing, but unused IP-Cores
should not be disabled in DT, so that hwmod picks them up
for power management.

Note, that it actually is disabled in omap3.dtsi and then enabled in
omap34xx/omap36xx dts files (the other variants do not have an ssi
module).

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 2/2] ARM: dts: omap3: keep ssi ports by default

[Roger Quadros]

[-- Attachment #1: Type: text/plain, Size: 1021 bytes --]

On 14/10/15 14:37, Sebastian Reichel wrote:
> Hi,
> 
> On Wed, Oct 14, 2015 at 02:27:27PM +0300, Roger Quadros wrote:
>> On 14/10/15 14:19, Sebastian Reichel wrote:
>>> On Wed, Oct 14, 2015 at 01:44:16PM +0300, Roger Quadros wrote:
>>>> Let's keep the SSI ports disabled in the omap3.dtsi to avoid
>>>> getting the following noise on the console for boards that don't
>>>> use the SSI ports.
>>>>
>>>> "omap_ssi_port 4805a000.ssi-port: DT data is missing cawake gpio (err=-2)"
>>>>
>>>> As omap3-n900 uses one SSI port, mark it enabled there.
>>
>> Would it be preferable to disable the ssi-controller node as well in the
>> omap3.dtsi file?
> 
> If I remember it correctly, existing, but unused IP-Cores
> should not be disabled in DT, so that hwmod picks them up
> for power management.

OK.
> 
> Note, that it actually is disabled in omap3.dtsi and then enabled in
> omap34xx/omap36xx dts files (the other variants do not have an ssi
> module).

Thanks for the info :)

cheers,
-roger


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 2/2] ARM: dts: omap3: keep ssi ports by default

[Tony Lindgren]

* Roger Quadros <rogerq@ti.com> [151014 03:48]:
> Let's keep the SSI ports disabled in the omap3.dtsi to avoid
> getting the following noise on the console for boards that don't
> use the SSI ports.
> 
> "omap_ssi_port 4805a000.ssi-port: DT data is missing cawake gpio (err=-2)"
> 
> As omap3-n900 uses one SSI port, mark it enabled there.

This is the wrong way around.. The SSI ports are there on all omap3 SoCs,
we do want to have the struct device created for them. It's best to have
the driver probe and then idle SSI and bail out. Marking something with
status = "disabled" should be only used if you really want to avoid kernel
touching the device at all, for example, if something is managed from
the start by a copropcessor.

Regards,

Tony


> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>  arch/arm/boot/dts/omap3-n900.dts | 1 +
>  arch/arm/boot/dts/omap3.dtsi     | 2 ++
>  2 files changed, 3 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
> index 5f5e0f3..bdb72fb 100644
> --- a/arch/arm/boot/dts/omap3-n900.dts
> +++ b/arch/arm/boot/dts/omap3-n900.dts
> @@ -949,6 +949,7 @@
>  };
>  
>  &ssi_port1 {
> +	status = "ok";
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&ssi_pins>;
>  
> diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
> index 8a2b253..25b0b37 100644
> --- a/arch/arm/boot/dts/omap3.dtsi
> +++ b/arch/arm/boot/dts/omap3.dtsi
> @@ -815,6 +815,7 @@
>  				interrupt-parent = <&intc>;
>  				interrupts = <67>,
>  					     <68>;
> +				status = "disabled";
>  			};
>  
>  			ssi_port2: ssi-port@4805b000 {
> @@ -828,6 +829,7 @@
>  				interrupt-parent = <&intc>;
>  				interrupts = <69>,
>  					     <70>;
> +				status = "disabled";
>  			};
>  		};
>  	};
> -- 
> 2.1.4
> 

Re: [PATCH 2/2] ARM: dts: omap3: keep ssi ports by default

[Roger Quadros]

On 19/10/15 19:01, Tony Lindgren wrote:
> * Roger Quadros <rogerq@ti.com> [151014 03:48]:
>> Let's keep the SSI ports disabled in the omap3.dtsi to avoid
>> getting the following noise on the console for boards that don't
>> use the SSI ports.
>>
>> "omap_ssi_port 4805a000.ssi-port: DT data is missing cawake gpio (err=-2)"
>>
>> As omap3-n900 uses one SSI port, mark it enabled there.
> 
> This is the wrong way around.. The SSI ports are there on all omap3 SoCs,
> we do want to have the struct device created for them. It's best to have
> the driver probe and then idle SSI and bail out. Marking something with
> status = "disabled" should be only used if you really want to avoid kernel
> touching the device at all, for example, if something is managed from
> the start by a copropcessor.

Agreed. Please ignore this patch.

cheers,
-roger

> 
> 
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>  arch/arm/boot/dts/omap3-n900.dts | 1 +
>>  arch/arm/boot/dts/omap3.dtsi     | 2 ++
>>  2 files changed, 3 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
>> index 5f5e0f3..bdb72fb 100644
>> --- a/arch/arm/boot/dts/omap3-n900.dts
>> +++ b/arch/arm/boot/dts/omap3-n900.dts
>> @@ -949,6 +949,7 @@
>>  };
>>  
>>  &ssi_port1 {
>> +	status = "ok";
>>  	pinctrl-names = "default";
>>  	pinctrl-0 = <&ssi_pins>;
>>  
>> diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
>> index 8a2b253..25b0b37 100644
>> --- a/arch/arm/boot/dts/omap3.dtsi
>> +++ b/arch/arm/boot/dts/omap3.dtsi
>> @@ -815,6 +815,7 @@
>>  				interrupt-parent = <&intc>;
>>  				interrupts = <67>,
>>  					     <68>;
>> +				status = "disabled";
>>  			};
>>  
>>  			ssi_port2: ssi-port@4805b000 {
>> @@ -828,6 +829,7 @@
>>  				interrupt-parent = <&intc>;
>>  				interrupts = <69>,
>>  					     <70>;
>> +				status = "disabled";
>>  			};
>>  		};
>>  	};
>> -- 
>> 2.1.4
>>