linux-kernel - Re: [PATCH v2] phy: Add a driver for dm816x USB PHY

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Wed, 18 Mar 2015 08:26:19 -0700
From:	Tony Lindgren <tony@...mide.com>
To:	Kishon Vijay Abraham I <kishon@...com>
Cc:	linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
	linux-omap@...r.kernel.org, Bin Liu <binmlist@...il.com>,
	Brian Hutchinson <b.hutchman@...il.com>,
	Felipe Balbi <balbi@...com>,
	Matthijs van Duin <matthijsvanduin@...il.com>,
	Paul Bolle <pebolle@...cali.nl>,
	Rusty Russell <rusty@...tcorp.com.au>
Subject: Re: [PATCH v2] phy: Add a driver for dm816x USB PHY

* Kishon Vijay Abraham I <kishon@...com> [150318 08:09]:
> On Wednesday 18 March 2015 05:42 AM, Tony Lindgren wrote:
> >+
> >+/*
> >+ * We have phy-core.c handle pm_runtime calls for us. We implement
> >+ * these functions for phy-core.c to keep track of power_count.
> >+ * Note that we may want to remove these eventually and rely only
> >+ * on the usecounting done by PM runtime.
> 
> irrespective of whether the phy drivers implement power_off/power_on callbacks
> or not, the phy core maintains it's usecount. So the following two functions
> shouldn't be needed at all.

OK thanks, I will drop them then.

> >+static int dm816x_usb_phy_power_off(struct phy *x)
> >+{
> >+	return 0;
> >+}
> >+
> >+static int dm816x_usb_phy_power_on(struct phy *x)
> >+{
> >+	return 0;
> >+}
...

> >+static int dm816x_usb_phy_runtime_suspend(struct device *dev)
> >+{
> >+	struct dm816x_usb_phy *phy = dev_get_drvdata(dev);
> >+	unsigned int mask, val;
> >+	int error = 0;
> >+
> >+	mask = BIT(phy->instance);
> >+	val = ~BIT(phy->instance);
> >+	error = regmap_update_bits(phy->syscon, phy->usb_ctrl,
> >+				   mask, val);
> 
> Shouldn't this be protected since both the PHYs can access the same register?

Here regmap takes care of the locking of the syscon register.
Note that this only happens when using regmap_update_bits(), doing
a separate regmap read followed by a regmap write won't achieve the
same.

BTW, we should update the other PHY drivers accessing the SCM region
to do the same using the syscon if not doing already.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/