| Message ID | 547F3C96.9070509@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 12/03/2014 05:38 PM, Andrew Jackson wrote: [,,,[ > + dw_i2s_dai = devm_kzalloc(&pdev->dev, sizeof(*dw_i2s_dai), GFP_KERNEL); > + if (!dw_i2s_dai) { > + dev_err(&pdev->dev, "mem allocation failed for dai driver\n"); All the memory alloc functions already print a error message. > return -ENOMEM; > } > > + dw_i2s_dai->ops = &dw_i2s_dai_ops; > + dw_i2s_dai->suspend = dw_i2s_suspend; > + dw_i2s_dai->resume = dw_i2s_resume; This seems to be separate from the devm_ioremap_resource() change. > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) { You don't actually have to check it devm_ioremap_resource does this for you. > + dev_err(&pdev->dev, "no i2s resource defined\n"); > + return -ENODEV; > + } > + > + dev->i2s_base = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(dev->i2s_base)) { > + dev_err(&pdev->dev, "ioremap fail for i2s_region\n"); Same here devm_ioremap_resource() will already print a appropriate error message. > + return PTR_ERR(dev->i2s_base); > + }
On Wed, Dec 03, 2014 at 04:38:46PM +0000, Andrew Jackson wrote: > Prepare for the introduction of device-tree support by re-ordering some > of the allocations and using devm_iomap_resource to simplify IO mapping. Applied, thanks.
On 12/03/14 16:44, Lars-Peter Clausen wrote: > On 12/03/2014 05:38 PM, Andrew Jackson wrote: > [,,,[ >> + dw_i2s_dai = devm_kzalloc(&pdev->dev, sizeof(*dw_i2s_dai), GFP_KERNEL); >> + if (!dw_i2s_dai) { >> + dev_err(&pdev->dev, "mem allocation failed for dai driver\n"); > > All the memory alloc functions already print a error message. I will remove the error message(s). >> return -ENOMEM; >> } >> >> + dw_i2s_dai->ops = &dw_i2s_dai_ops; >> + dw_i2s_dai->suspend = dw_i2s_suspend; >> + dw_i2s_dai->resume = dw_i2s_resume; > > This seems to be separate from the devm_ioremap_resource() change. It seemed reasonable to assign these known values to the dw_i2s_dai as soon as it had been allocated. Would you prefer this in a separate patch? >> + >> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> + if (!res) { > > You don't actually have to check it devm_ioremap_resource does this for you. Oh, thanks, paranoia was taking over. >> + dev_err(&pdev->dev, "no i2s resource defined\n"); >> + return -ENODEV; >> + } >> + >> + dev->i2s_base = devm_ioremap_resource(&pdev->dev, res); >> + if (IS_ERR(dev->i2s_base)) { >> + dev_err(&pdev->dev, "ioremap fail for i2s_region\n"); > > Same here devm_ioremap_resource() will already print a appropriate error > message. > >> + return PTR_ERR(dev->i2s_base); >> + } > >
On 12/03/14 18:26, Mark Brown wrote: > On Wed, Dec 03, 2014 at 04:38:46PM +0000, Andrew Jackson wrote: >> Prepare for the introduction of device-tree support by re-ordering some >> of the allocations and using devm_iomap_resource to simplify IO mapping. > > Applied, thanks. > Lars-Peter Clausen had some comments on this patch so I was intending to resubmit it. Would you prefer that I submit a patch to my original? Andrew
On Thu, Dec 04, 2014 at 09:05:01AM +0000, Andrew Jackson wrote: > Lars-Peter Clausen had some comments on this patch so I was intending > to resubmit it. Would you prefer that I submit a patch to my > original? Incremental patches, same as is standard for upstream. Please also fix your mail client to word wrap within paragraphs so your mails are readable.
diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c index e961388..08f0229 100644 --- a/sound/soc/dwc/designware_i2s.c +++ b/sound/soc/dwc/designware_i2s.c @@ -338,31 +338,34 @@ static int dw_i2s_probe(struct platform_device *pdev) return -EINVAL; } - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (!res) { - dev_err(&pdev->dev, "no i2s resource defined\n"); - return -ENODEV; - } - - if (!devm_request_mem_region(&pdev->dev, res->start, - resource_size(res), pdev->name)) { - dev_err(&pdev->dev, "i2s region already claimed\n"); - return -EBUSY; - } - dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL); if (!dev) { dev_warn(&pdev->dev, "kzalloc fail\n"); return -ENOMEM; } - dev->i2s_base = devm_ioremap(&pdev->dev, res->start, - resource_size(res)); - if (!dev->i2s_base) { - dev_err(&pdev->dev, "ioremap fail for i2s_region\n"); + dw_i2s_dai = devm_kzalloc(&pdev->dev, sizeof(*dw_i2s_dai), GFP_KERNEL); + if (!dw_i2s_dai) { + dev_err(&pdev->dev, "mem allocation failed for dai driver\n"); return -ENOMEM; } + dw_i2s_dai->ops = &dw_i2s_dai_ops; + dw_i2s_dai->suspend = dw_i2s_suspend; + dw_i2s_dai->resume = dw_i2s_resume; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) { + dev_err(&pdev->dev, "no i2s resource defined\n"); + return -ENODEV; + } + + dev->i2s_base = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(dev->i2s_base)) { + dev_err(&pdev->dev, "ioremap fail for i2s_region\n"); + return PTR_ERR(dev->i2s_base); + } + cap = pdata->cap; dev->capability = cap; dev->i2s_clk_cfg = pdata->i2s_clk_cfg; @@ -388,13 +391,6 @@ static int dw_i2s_probe(struct platform_device *pdev) if (ret < 0) goto err_clk_put; - dw_i2s_dai = devm_kzalloc(&pdev->dev, sizeof(*dw_i2s_dai), GFP_KERNEL); - if (!dw_i2s_dai) { - dev_err(&pdev->dev, "mem allocation failed for dai driver\n"); - ret = -ENOMEM; - goto err_clk_disable; - } - if (cap & DWC_I2S_PLAY) { dev_dbg(&pdev->dev, " designware: play supported\n"); dw_i2s_dai->playback.channels_min = MIN_CHANNEL_NUM; @@ -411,10 +407,6 @@ static int dw_i2s_probe(struct platform_device *pdev) dw_i2s_dai->capture.rates = pdata->snd_rates; } - dw_i2s_dai->ops = &dw_i2s_dai_ops; - dw_i2s_dai->suspend = dw_i2s_suspend; - dw_i2s_dai->resume = dw_i2s_resume; - dev->dev = &pdev->dev; dev_set_drvdata(&pdev->dev, dev); ret = snd_soc_register_component(&pdev->dev, &dw_i2s_component,
Prepare for the introduction of device-tree support by re-ordering some of the allocations and using devm_iomap_resource to simplify IO mapping. Signed-off-by: Andrew Jackson <Andrew.Jackson@arm.com> --- sound/soc/dwc/designware_i2s.c | 46 ++++++++++++++++----------------------- 1 files changed, 19 insertions(+), 27 deletions(-)