Re: [PATCH v3 4/6] reset: Add Delta TN48M CPLD reset controller

On Wed, Jun 2, 2021 at 10:47 AM Philipp Zabel [off-list ref] wrote:

On Tue, 2021-06-01 at 19:09 +0200, Robert Marko wrote:
[...]

quoted

Yes, it's self-clearing, per spec they will be cleared after 100ms.

Can you make sure the function only returns after the reset is
deasserted again, for example by using regmap_read_poll_timeout() on the
reset bit?

Yes, that is simple to implement.

quoted

Will drop assert then, I saw that reset was for self-clearing, but other
drivers I looked for example implemented both which was confusing.

If you have full control over the reset line, you can implement .reset
by manually asserting and deasserting (possibly after a delay). But if
the reset is self-clearing, you can't properly implement .(de)assert,
which have an expectation about the state of the reset line after the
function returns.

Makes sense, I will drop the assert op.

quoted

quoted

quoted

+}
+
+static int tn48m_control_status(struct reset_controller_dev *rcdev,
+                             unsigned long id)
+{
+     struct tn48_reset_data *data = to_tn48_reset_data(rcdev);
+     unsigned int regval;
+     int ret;
+
+     ret = regmap_read(data->regmap, TN48M_RESET_REG, &regval);
+     if (ret < 0)
+             return ret;
+
+     if (BIT(tn48m_resets[id].bit) & regval)
+             return 0;
+     else
+             return 1;
+}
+
+static const struct reset_control_ops tn48_reset_ops = {
+     .reset          = tn48m_control_reset,
+     .assert         = tn48m_control_assert,
+     .status         = tn48m_control_status,
+};
+
+static int tn48m_reset_probe(struct platform_device *pdev)
+{
+     struct tn48_reset_data *data;
+     struct regmap *regmap;
+
+     if (!pdev->dev.parent)
+             return -ENODEV;

That shouldn't be necessary.

This driver depends on having a parent as it needs to get the
regmap from it.
The parent is a CPLD using simple-i2c-mfd.
So it's nice to check.

pdev->dev.parent is always set to &platform_bus if there is no parent.

Ok, so it's useless to check.

Will send a new version today.

Regards,
Robert

regards
Philipp

-- 
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: robert.marko@sartura.hr
Web: www.sartura.hr