On Mon, Nov 16, 2015 at 11:42 PM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> On Sun, 15 Nov 2015, Hans de Goede wrote:
>
>> From: Reinder de Haan <patchesrdh@xxxxxxxxx>
>>
>> At least the EHCI found on the Allwinnner H3 SoC needs multiple reset
>> lines, the controller will not initialize while the reset for its
>> companion OHCI is still asserted, which means we need to de-assert
>> 2 reset-controllers for this EHCI controller to work.
>
> I assume that reset_control_deassert() is smart enough to maintain a
> count of de-assertions, and it doesn't actually turn on the reset line
> until the count drops to 0. Right?
No it doesn't. That might be a problem when 2 devices (such as EHCI / OHCI
pairs) share a reset line, one probes successfully while the other doesn't.
Hans?
>> Signed-off-by: Reinder de Haan <patchesrdh@xxxxxxxxx>
>> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
>
> ...
>
>> @@ -229,18 +230,24 @@ static int ehci_platform_probe(struct platform_device *dev)
>> break;
>> }
>> }
>> - }
>>
>> - priv->rst = devm_reset_control_get_optional(&dev->dev, NULL);
>> - if (IS_ERR(priv->rst)) {
>> - err = PTR_ERR(priv->rst);
>> - if (err == -EPROBE_DEFER)
>> - goto err_put_clks;
>> - priv->rst = NULL;
>> - } else {
>> - err = reset_control_deassert(priv->rst);
>> - if (err)
>> - goto err_put_clks;
>> + for (rst = 0; rst < EHCI_MAX_RESETS; rst++) {
>
> What happens on platforms that don't use OF? Or if pdata is not equal
> to &ehci_platform_defaults? Can you guarantee that those platforms
> will never need to turn off a reset line?
The reset control framework is OF / DT only at the moment.
Regards
ChenYu
>> + priv->resets[rst] =
>> + of_reset_control_get_by_index(dev->dev.of_node,
>> + rst);
>
> The style used in this file is to indent continuation lines to 2 two
> stops, not to line things up with an open paren on the previous line.
>
> The rest of the patch looks okay.
>
> Alan Stern
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
