On Thu, Oct 27, 2011 at 14:48, Linus Walleij wrote:
> Platforms that use regulators and the smsc911x and have no defined
> regulator for the smsc911x and claim complete regulator
> constraints with no dummy regulators will need to provide it, for
> example using a fixed voltage regulator. It appears that this may
> affect (apart from Ux500 Snowball) possibly these archs/machines
> that from some grep:s appear to define both CONFIG_SMSC911X and
> CONFIG_REGULATOR:
>
> - ARM Freescale mx3 and OMAP 2 plus, Raumfeld machines
> - Blackfin
> - Super-H
no Blackfin board in the tree uses regulators by default. we do list
regulator resources with some boards so people can rebuild the
development system to support addon daughter cards.
my gut reaction: smsc911x is working just fine without regulator
support for many people, so why do we suddenly need to make it a
requirement ? this is a fairly small amount of code, so adding a
smsc911x Kconfig symbol to control the regulator support seems like
overkill. only other option would be to change the patch to not make
missing regulators non-fatal. so i'd probably lean towards the latter
(and it sounds like you changed this with earlier versions).
> --- a/drivers/net/ethernet/smsc/smsc911x.c
> +++ b/drivers/net/ethernet/smsc/smsc911x.c
>
> +#define SMSC911X_NUM_SUPPLIES 2
this gets used once (to define the array), so i wonder if it shouldn't
just be inlined
> +static int smsc911x_enable_disable_resources(struct platform_device *pdev,
> + bool enable)
> +{
> ...
> + if (enable) {
> + ret = regulator_bulk_enable(ARRAY_SIZE(pdata->supplies),
> + pdata->supplies);
> + if (ret)
> + netdev_err(ndev, "failed to enable regulators %d\n",
> + ret);
> ...
> static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
> ...
> + retval = smsc911x_request_free_resources(pdev, true);
> + if (retval) {
> + pr_err("Could request regulators needed aborting\n");
you warn twice here, and the grammar in the later error is broken, and
uses pr_err() instead of netdev_err(). i would simply drop the latter
pr_err().
> +static int smsc911x_request_free_resources(struct platform_device *pdev,
> + bool request)
> +{
> ...
> + if (request) {
> + ret = regulator_bulk_get(&pdev->dev,
> + ARRAY_SIZE(pdata->supplies),
> + pdata->supplies);
> + if (ret)
> + netdev_err(ndev, "couldn't get regulators %d\n",
> + ret);
> static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
> ...
> + retval = smsc911x_enable_disable_resources(pdev, true);
> + if (retval) {
> + pr_err("Could enable regulators needed aborting\n");
same exact issues with the request/free helper
> + retval = smsc911x_request_free_resources(pdev, false);
> + /* ignore not all have regulators */
old comment ?
-mike
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
