Re: [PATCH V6 02/10] ASoC: SAMSUNG: Add DT support for i2s

[Tomasz Figa <t.figa@samsung.com>]

Hi,

On Thursday 14 of February 2013 11:48:59 Sylwester Nawrocki wrote:
> On 01/18/2013 12:47 PM, Padmavathi Venna wrote:
> > Add support for device based discovery.
> > 
> > Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
> > ---
> 
> ...
> 
> >  /* Lock for cross i/f checks */
> > 
> > @@ -997,19 +1006,76 @@ static struct i2s_dai *i2s_alloc_dai(struct
> > platform_device *pdev, bool sec)> 
> >  	return i2s;
> >  
> >  }
> > 
> > +#ifdef CONFIG_OF
> > +static int samsung_i2s_parse_dt_gpio(struct i2s_dai *i2s)
> > +{
> > +	struct device *dev = &i2s->pdev->dev;
> > +	int index, gpio, ret;
> > +
> > +	for (index = 0; index < 7; index++) {
> > +		gpio = of_get_gpio(dev->of_node, index);
> > +		if (!gpio_is_valid(gpio)) {
> > +			dev_err(dev, "invalid gpio[%d]: %d\n", index, gpio);
> > +			goto free_gpio;
> > +		}
> > +
> > +		ret = gpio_request(gpio, dev_name(dev));
> > +		if (ret) {
> > +			dev_err(dev, "gpio [%d] request failed\n", gpio);
> > +			goto free_gpio;
> > +		}
> > +		i2s->gpios[index] = gpio;
> > +	}
> > +	return 0;
> > +
> > +free_gpio:
> > +	while (--index >= 0)
> > +		gpio_free(i2s->gpios[index]);
> > +	return -EINVAL;
> > +}
> > +
> > +static void samsung_i2s_dt_gpio_free(struct i2s_dai *i2s)
> > +{
> > +	unsigned int index;
> > +	for (index = 0; index < 7; index++)
> > +		gpio_free(i2s->gpios[index]);
> > +}
> > +#else
> > +static int samsung_i2s_parse_dt_gpio(struct i2s_dai *dai)
> > +{
> > +	return -EINVAL;
> > +}
> > +
> > +static void samsung_i2s_dt_gpio_free(struct i2s_dai *dai)
> > +{
> > +}
> > +
> > +#endif
> 
> NAK.

+1

> 
> Why we should leave with this temporary code when there is already
> pinctrl support for Exynos SoCs ?
> 
> If the pinctrl driver for Exynos5 was updated you could instead just do
> 
> devm_pinctrl_get_select_default(&i2s->pdev->dev);

I said it already by the way of some other Exynos5 patches that Exynos5 
has to be migrated to pin control.

The legacy GPIO interface was supposed to be kept only for compatibility 
with existing code (until it gets migrated to pin control), not for adding 
new code relying on it.

If you intend to add new code which needs to configure pin muxes, you 
should migrate Exynos5 to pin control first. This won't be particularly 
hard, since pinctrl-exynos driver already supports Exynos5 and all that is 
missing are Device Tree nodes for it.

Best regards,
-- 
Tomasz Figa
Samsung Poland R&D Center
SW Solution Development, Linux Platform