On Thu, Jan 21, 2016 at 6:56 PM, Johan Hovold johan@kernel.org wrote:
On Thu, Jan 21, 2016 at 05:27:58PM +0100, Michael Trimarchi wrote:
Hi
On Thu, Jan 21, 2016 at 4:36 PM, Johan Hovold johan@kernel.org wrote:
On Thu, Jan 21, 2016 at 04:26:56PM +0100, Jacob Siverskog wrote:
The pcm179x family supports both SPI and I2C for configuration. This patch splits the driver into core and SPI parts, in preparation for I2C support.
Reviewed-by: Johan Hovold johan@kernel.org Signed-off-by: Jacob Siverskog jacob@teenage.engineering
diff --git a/sound/soc/codecs/pcm179x-spi.c b/sound/soc/codecs/pcm179x-spi.c new file mode 100644 index 0000000..5842add9 --- /dev/null +++ b/sound/soc/codecs/pcm179x-spi.c
-static int pcm179x_spi_probe(struct spi_device *spi) +int pcm179x_common_init(struct device *dev, struct regmap *regmap) { struct pcm179x_private *pcm179x; int ret;
pcm179x = devm_kzalloc(&spi->dev, sizeof(struct pcm179x_private),
if (IS_ERR(regmap)) {ret = PTR_ERR(regmap);dev_err(dev, "Failed to register regmap: %d\n", ret);return ret;}This looks weird. I think you should check for error where you do the allocation even if this means a four more lines of code in total.
agree on that
Ok.
pcm179x = devm_kzalloc(dev, sizeof(struct pcm179x_private), GFP_KERNEL); if (!pcm179x) return -ENOMEM;
spi_set_drvdata(spi, pcm179x);pcm179x->regmap = devm_regmap_init_spi(spi, &pcm179x_regmap);if (IS_ERR(pcm179x->regmap)) {ret = PTR_ERR(pcm179x->regmap);dev_err(&spi->dev, "Failed to register regmap: %d\n", ret);return ret;}
pcm179x->regmap = regmap;dev_set_drvdata(dev, pcm179x);snd_soc_codec_set_drvdata
Is this more "codec" like?
I'd say, only if you also add a codec probe callback and use it from there. The other codec drivers appear to be consistent on that.
I agree. Using snd_soc_codec_set_drvdata would mean larger logical changes that I don't really see the purpose of doing. All other codec drivers that are separated in a similar fashion as this patch use dev_set_drvdata.