Hi Philipp,
2016-07-27 18:17 GMT+09:00 Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>:
>> +
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +#include <linux/reset-controller.h>
>> +
>> +#include "reset-uniphier.h"
>> +
>> +struct uniphier_reset_priv {
>> + struct reset_controller_dev rcdev;
>> + struct device *dev;
>> + struct regmap *regmap;
>> + const struct uniphier_reset_data *data;
>> +};
>> +
>> +#define to_uniphier_reset_priv(_rcdev) \
>> + container_of(_rcdev, struct uniphier_reset_priv, rcdev)
>> +
>> +static int uniphier_reset_update(struct reset_controller_dev *rcdev,
>> + unsigned long id, bool assert)
>> +{
>> + struct uniphier_reset_priv *priv = to_uniphier_reset_priv(rcdev);
>> + const struct uniphier_reset_data *p;
>> + bool handled = false;
>> +
>> + for (p = priv->data; p->id != UNIPHIER_RESET_ID_END; p++) {
>> + unsigned int val;
>> + int ret;
>> +
>> + if (p->id != id)
>> + continue;
>> +
>> + val = p->assert_val;
>> + if (!assert)
>> + val = ~val;
>> +
>> + ret = regmap_write_bits(priv->regmap, p->reg, p->mask, val);
>> + if (ret)
>> + return ret;
>> +
>> + handled = true;
>
> Why does this continue to walk through the list after the correct id was
> found?
Looks like you already found the answer for this.
>> +#define UNIPHIER_MIO_RESET_USB2(index, ch) \
>> + UNIPHIER_RESETX_SIMPLE((index), 0x110 + 0x200 * (ch), BIT(24)), \
>> + UNIPHIER_RESETX_SIMPLE((index), 0x114 + 0x200 * (ch), BIT(0))
>
> Ah, so for USB2 reset you have two reset bits in separate registers. Are
> you sure these are controlling the same reset line?
I am not a hardware guy, so I am not sure about the hardware design.
>From my best guess, I think each bit controls a different block.
But both of them must be de-asserted before starting up USB.
There is no use-case where they are asserted/de-asserted independently.
So, I thought it made sense to couple them into a single ID.
> If the USB core does in fact have two separate reset inputs that just
> happen to need asserting at the same time, this should still get two
> separate ids. Same issue for the SD reset above, if the reset lines are
> physically separate, please don't combine them in the driver.
Right.
>From the view of point of Device Tree interface,
it should reflect the hardware design.
I believe they are separate reset signals, so should be given with separate IDs.
But, as a software engineer, it is sometimes difficult to fully understand
the hardware structure.
The hardware document often just says "how to use USB",
but "how clock/reset signals are connected in each block" is not mentioned,
or at least very unclear.
Probably, I will come back with real per-reset-line ID,
but I need some time to take a look.
--
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
