From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> To: Daniel Mack <daniel@zonque.org>, linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org, linux-i2c@vger.kernel.org, alsa-devel@alsa-project.org, devicetree@vger.kernel.org, linux-clk@vger.kernel.org Cc: lars@metafoo.de, sboyd@kernel.org, mturquette@baylibre.com, robh+dt@kernel.org, broonie@kernel.org, pascal.huerst@gmail.com, lee.jones@linaro.org Subject: Re: [alsa-devel] [PATCH 06/10] mfd: Add core driver for AD242x A2B transceivers Date: Tue, 17 Dec 2019 13:16:04 -0600 [thread overview] Message-ID: <c3885b01-e6ec-1b67-6e48-69e3fc40bb62@linux.intel.com> (raw) In-Reply-To: <20191209183511.3576038-8-daniel@zonque.org> > +config MFD_AD242X > + bool "Analog Devices AD242x A2B support" > + select MFD_CORE > + select REGMAP_I2C > + depends on I2C=y && OF is there a specific reason why I2C needs to be built-in (as opposed to 'm')? > +/* See Table 3-2 in the datasheet */ is the datasheet public? I thought it was only available under NDA. > + master->sync_clk = devm_clk_get(dev, "sync"); > + if (IS_ERR(master->sync_clk)) { > + ret = PTR_ERR(master->sync_clk); > + if (ret != -EPROBE_DEFER) > + dev_err(dev, "failed to get sync clk: %d\n", ret); > + > + return ret; > + } > + > + if (of_property_read_u32(dev->of_node, "clock-frequency", > + &master->sync_clk_rate)) { > + ret = clk_set_rate(master->sync_clk, master->sync_clk_rate); shouldn't you check the rate before setting it? > + if (ret < 0) { > + dev_err(dev, "Cannot set sync clock rate: %d\n", ret); > + return ret; > + } > + } > + > + master->sync_clk_rate = clk_get_rate(master->sync_clk); > + if (master->sync_clk_rate != 44100 && master->sync_clk_rate != 48000) { > + dev_err(dev, "SYNC clock rate %d is invalid\n", > + master->sync_clk_rate); > + return -EINVAL; > + } this is a bit odd, you set the rate in case there is a property but get it anyways. the last block could be an else? > + > + ret = clk_prepare_enable(master->sync_clk); > + if (ret < 0) { > + dev_err(dev, "failed to enable sync clk: %d\n", ret); > + return ret; > + } > + > + /* Master node setup */ > + > + ret = regmap_write(regmap, AD242X_CONTROL, > + AD242X_CONTROL_MSTR | AD242X_CONTROL_SOFTRST); > + if (ret < 0) > + return ret; > + > + ret = ad242x_wait_for_irq(master, &master->run_completion, 10); what is 10? > +static int ad242x_master_remove(struct i2c_client *i2c) > +{ > + struct ad242x_master *master = i2c_get_clientdata(i2c); > + > + if (master->sync_clk) > + clk_disable_unprepare(master->sync_clk); earlier you tested for IS_ERR(master->sync_clk)? > + for (i = 0; i < 4; i++) { what is 4? 4 hops?
WARNING: multiple messages have this Message-ID (diff)
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> To: Daniel Mack <daniel@zonque.org>, linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org, linux-i2c@vger.kernel.org, alsa-devel@alsa-project.org, devicetree@vger.kernel.org, linux-clk@vger.kernel.org Cc: lars@metafoo.de, sboyd@kernel.org, mturquette@baylibre.com, robh+dt@kernel.org, broonie@kernel.org, pascal.huerst@gmail.com, lee.jones@linaro.org Subject: Re: [alsa-devel] [PATCH 06/10] mfd: Add core driver for AD242x A2B transceivers Date: Tue, 17 Dec 2019 13:16:04 -0600 [thread overview] Message-ID: <c3885b01-e6ec-1b67-6e48-69e3fc40bb62@linux.intel.com> (raw) In-Reply-To: <20191209183511.3576038-8-daniel@zonque.org> > +config MFD_AD242X > + bool "Analog Devices AD242x A2B support" > + select MFD_CORE > + select REGMAP_I2C > + depends on I2C=y && OF is there a specific reason why I2C needs to be built-in (as opposed to 'm')? > +/* See Table 3-2 in the datasheet */ is the datasheet public? I thought it was only available under NDA. > + master->sync_clk = devm_clk_get(dev, "sync"); > + if (IS_ERR(master->sync_clk)) { > + ret = PTR_ERR(master->sync_clk); > + if (ret != -EPROBE_DEFER) > + dev_err(dev, "failed to get sync clk: %d\n", ret); > + > + return ret; > + } > + > + if (of_property_read_u32(dev->of_node, "clock-frequency", > + &master->sync_clk_rate)) { > + ret = clk_set_rate(master->sync_clk, master->sync_clk_rate); shouldn't you check the rate before setting it? > + if (ret < 0) { > + dev_err(dev, "Cannot set sync clock rate: %d\n", ret); > + return ret; > + } > + } > + > + master->sync_clk_rate = clk_get_rate(master->sync_clk); > + if (master->sync_clk_rate != 44100 && master->sync_clk_rate != 48000) { > + dev_err(dev, "SYNC clock rate %d is invalid\n", > + master->sync_clk_rate); > + return -EINVAL; > + } this is a bit odd, you set the rate in case there is a property but get it anyways. the last block could be an else? > + > + ret = clk_prepare_enable(master->sync_clk); > + if (ret < 0) { > + dev_err(dev, "failed to enable sync clk: %d\n", ret); > + return ret; > + } > + > + /* Master node setup */ > + > + ret = regmap_write(regmap, AD242X_CONTROL, > + AD242X_CONTROL_MSTR | AD242X_CONTROL_SOFTRST); > + if (ret < 0) > + return ret; > + > + ret = ad242x_wait_for_irq(master, &master->run_completion, 10); what is 10? > +static int ad242x_master_remove(struct i2c_client *i2c) > +{ > + struct ad242x_master *master = i2c_get_clientdata(i2c); > + > + if (master->sync_clk) > + clk_disable_unprepare(master->sync_clk); earlier you tested for IS_ERR(master->sync_clk)? > + for (i = 0; i < 4; i++) { what is 4? 4 hops? _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
next prev parent reply other threads:[~2019-12-17 19:54 UTC|newest] Thread overview: 83+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-12-09 18:35 [PATCH 00/10] mfd: Add support for Analog Devices A2B transceiver Daniel Mack 2019-12-09 18:35 ` [alsa-devel] " Daniel Mack 2019-12-09 18:35 ` Daniel Mack 2019-12-09 18:35 ` [PATCH 01/10] dt-bindings: mfd: Add documentation for ad242x Daniel Mack 2019-12-09 18:35 ` [alsa-devel] " Daniel Mack 2019-12-09 18:35 ` Daniel Mack 2019-12-19 19:29 ` Rob Herring 2019-12-19 19:29 ` [alsa-devel] " Rob Herring 2019-12-09 18:35 ` [PATCH 02/10] dt-bindings: i2c: Add documentation for ad242x i2c controllers Daniel Mack 2019-12-09 18:35 ` [alsa-devel] " Daniel Mack 2019-12-09 18:35 ` Daniel Mack 2020-01-08 3:45 ` Rob Herring 2020-01-08 3:45 ` [alsa-devel] " Rob Herring 2019-12-09 18:35 ` [PATCH 03/10] dt-bindings: gpio: Add documentation for ad242x GPIO controllers Daniel Mack 2019-12-09 18:35 ` [alsa-devel] " Daniel Mack 2019-12-09 18:35 ` Daniel Mack 2019-12-09 18:35 ` [PATCH 03/10] dt-bindings: gpio: Add documentation for AD242x " Daniel Mack 2019-12-09 18:35 ` [alsa-devel] " Daniel Mack 2019-12-09 18:35 ` Daniel Mack 2019-12-09 18:35 ` [PATCH 04/10] dt-bindings: clock: Add documentation for AD242x clock providers Daniel Mack 2019-12-09 18:35 ` [alsa-devel] " Daniel Mack 2019-12-09 18:35 ` Daniel Mack 2019-12-24 7:32 ` Stephen Boyd 2019-12-24 7:32 ` [alsa-devel] " Stephen Boyd 2019-12-24 7:32 ` Stephen Boyd 2019-12-09 18:35 ` [PATCH 05/10] dt-bindings: sound: Add documentation for AD242x codecs Daniel Mack 2019-12-09 18:35 ` [alsa-devel] " Daniel Mack 2019-12-09 18:35 ` Daniel Mack 2019-12-09 18:35 ` [PATCH 06/10] mfd: Add core driver for AD242x A2B transceivers Daniel Mack 2019-12-09 18:35 ` [alsa-devel] " Daniel Mack 2019-12-17 13:39 ` Lee Jones 2019-12-17 13:39 ` [alsa-devel] " Lee Jones 2019-12-17 13:46 ` Lee Jones 2019-12-17 13:46 ` [alsa-devel] " Lee Jones 2019-12-17 19:36 ` Daniel Mack 2019-12-17 19:36 ` [alsa-devel] " Daniel Mack 2019-12-17 19:24 ` Daniel Mack 2019-12-17 19:24 ` [alsa-devel] " Daniel Mack 2019-12-18 11:20 ` Luca Ceresoli 2019-12-18 11:20 ` [alsa-devel] " Luca Ceresoli 2019-12-17 19:16 ` Pierre-Louis Bossart [this message] 2019-12-17 19:16 ` Pierre-Louis Bossart 2019-12-18 9:40 ` Daniel Mack 2019-12-18 9:40 ` Daniel Mack 2019-12-09 18:35 ` [PATCH 07/10] i2c: Add driver for AD242x bus controller Daniel Mack 2019-12-09 18:35 ` [alsa-devel] " Daniel Mack 2019-12-09 18:35 ` Daniel Mack 2019-12-12 16:11 ` Luca Ceresoli 2019-12-12 16:11 ` [alsa-devel] " Luca Ceresoli 2019-12-12 16:33 ` Wolfram Sang 2019-12-12 16:33 ` [alsa-devel] " Wolfram Sang 2019-12-15 20:27 ` Daniel Mack 2019-12-15 20:27 ` [alsa-devel] " Daniel Mack 2019-12-17 8:35 ` Luca Ceresoli 2019-12-17 8:35 ` [alsa-devel] " Luca Ceresoli 2019-12-17 18:17 ` Daniel Mack 2019-12-17 18:17 ` [alsa-devel] " Daniel Mack 2019-12-09 18:35 ` [PATCH 08/10] gpio: Add driver for AD242x GPIO controllers Daniel Mack 2019-12-09 18:35 ` [alsa-devel] " Daniel Mack 2019-12-09 18:35 ` Daniel Mack 2019-12-09 18:35 ` [PATCH 09/10] clk: Add support for AD242x clock output providers Daniel Mack 2019-12-09 18:35 ` [alsa-devel] " Daniel Mack 2019-12-09 18:35 ` Daniel Mack 2019-12-24 7:46 ` Stephen Boyd 2019-12-24 7:46 ` [alsa-devel] " Stephen Boyd 2019-12-24 7:46 ` Stephen Boyd 2019-12-09 18:35 ` [PATCH 10/10] ASoC: Add codec component for AD242x nodes Daniel Mack 2019-12-09 18:35 ` [alsa-devel] " Daniel Mack 2019-12-09 18:35 ` Daniel Mack 2019-12-16 14:23 ` Mark Brown 2019-12-16 14:23 ` [alsa-devel] " Mark Brown 2019-12-17 19:28 ` Pierre-Louis Bossart 2019-12-17 19:28 ` Pierre-Louis Bossart 2019-12-18 9:49 ` Daniel Mack 2019-12-18 9:49 ` Daniel Mack 2019-12-18 15:32 ` Pierre-Louis Bossart 2019-12-18 15:32 ` Pierre-Louis Bossart 2019-12-20 8:24 ` Daniel Mack 2019-12-20 15:18 ` Pierre-Louis Bossart 2019-12-17 19:29 ` [alsa-devel] [PATCH 00/10] mfd: Add support for Analog Devices A2B transceiver Pierre-Louis Bossart 2019-12-17 19:29 ` Pierre-Louis Bossart 2019-12-18 9:53 ` Daniel Mack 2019-12-18 9:53 ` Daniel Mack
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=c3885b01-e6ec-1b67-6e48-69e3fc40bb62@linux.intel.com \ --to=pierre-louis.bossart@linux.intel.com \ --cc=alsa-devel@alsa-project.org \ --cc=broonie@kernel.org \ --cc=daniel@zonque.org \ --cc=devicetree@vger.kernel.org \ --cc=lars@metafoo.de \ --cc=lee.jones@linaro.org \ --cc=linux-clk@vger.kernel.org \ --cc=linux-gpio@vger.kernel.org \ --cc=linux-i2c@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mturquette@baylibre.com \ --cc=pascal.huerst@gmail.com \ --cc=robh+dt@kernel.org \ --cc=sboyd@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.