On 07/12, Philipp Zabel wrote:

Hi Linus,

On Tue, 2017-07-11 at 14:26 +0200, Linus Walleij wrote:

quoted

commit e2860e1f62f2 ("serial: 8250_of: Add reset support")
introduced reset support for the 8250_of driver.

However it unconditionally uses the assert/deassert pair to
deassert reset on the device at probe and assert it at
remove. This does not work with systems that have a
self-deasserting reset controller, such as Gemini, that
recently added a reset controller.

As a result, the console will not probe on the Gemini with
this message:

Serial: 8250/16550 driver, 1 ports, IRQ sharing disabled
of_serial: probe of 42000000.serial failed with error -524

This (-ENOTSUPP) is the error code returned by the
deassert() operation on self-deasserting reset controllers.

To work around this, implement dummy .assert() and
.deassert() operations in the Gemini combined clock and
reset controller. This fixes the issue on this system.

Cc: Joel Stanley <joel@jms.id.au>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-serial at vger.kernel.org
Fixes: e2860e1f62f2 ("serial: 8250_of: Add reset support")
Signed-off-by: Linus Walleij <redacted>
---
This is the solution suggested by Philipp, I think.

It is what I suggested, yes, but now that I see it before me, I don't
think this is the proper solution either. Reason below:

quoted

---
 drivers/clk/clk-gemini.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/clk/clk-gemini.c b/drivers/clk/clk-gemini.c
index c391a49aaaff..b4cf2f699a21 100644
--- a/drivers/clk/clk-gemini.c
+++ b/drivers/clk/clk-gemini.c

@@ -237,6 +237,18 @@ static int gemini_reset(struct reset_controller_dev *rcdev,
 			    BIT(GEMINI_RESET_CPU1) | BIT(id));
 }
 
+static int gemini_reset_assert(struct reset_controller_dev *rcdev,
+			       unsigned long id)
+{
+	return 0;

This is valid behaviour for shared reset controls, as sharing users
don't mind whether the reset line is actually asserted after this call,
they just allow it.

For an exclusive reset control this should return an error though, as
the caller would expect the reset line to be asserted after this call.
Unfortunately the core does not provide information whether the reset
control is shared or exclusive to the reset drivers, and it could be
argued that the drivers shouldn't have to care. I suppose I'll have to
handle this in the core, after all. What do you think of the attached
patch?

Otherwise, as a regression fix, I think this would be ok. There isn't
going to be any driver on the Gemini platform that requests an exclusive
reset control and then calls reset_control_assert, expecting the reset
line to stay asserted.

Acked-by: Philipp Zabel <p.zabel@pengutronix.de>

I'll queue this up for clk-fixes once -rc1 is out.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project