On Thu, 02 Aug 2018, Srinivas Kandagatla wrote:
On 02/08/18 09:33, Lee Jones wrote:
On Fri, 27 Jul 2018, Srinivas Kandagatla wrote:
Qualcomm WCD9335 Codec is a standalone Hi-Fi audio codec IC, It has mulitple blocks like Soundwire controller, codec, Codec processing engine, ClassH controller, interrupt mux. It supports both I2S/I2C and SLIMbus audio interfaces.
This patch adds support to SLIMbus audio interface.
Signed-off-by: Srinivas Kandagatla srinivas.kandagatla@linaro.org
drivers/mfd/Kconfig | 18 ++ drivers/mfd/Makefile | 4 + drivers/mfd/wcd9335-core.c | 268 ++++++++++++++++ include/linux/mfd/wcd9335/registers.h | 580 ++++++++++++++++++++++++++++++++++ include/linux/mfd/wcd9335/wcd9335.h | 42 +++ 5 files changed, 912 insertions(+) create mode 100644 drivers/mfd/wcd9335-core.c create mode 100644 include/linux/mfd/wcd9335/registers.h create mode 100644 include/linux/mfd/wcd9335/wcd9335.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index f3fa516011ec..6e5b5f3cfe20 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -1807,6 +1807,24 @@ config MFD_WM97xx support for the WM97xx, in order to use the actual functionaltiy of the device other drivers must be enabled. +config MFD_WCD9335
- tristate
- select MFD_CORE
- select REGMAP
- select REGMAP_IRQ
+config MFD_WCD9335_SLIM
- tristate "Qualcomm WCD9335 with SLIMbus"
- select MFD_WCD9335
- select REGMAP_SLIMBUS
- depends on SLIMBUS
- help
- The WCD9335 is a standalone Hi-Fi audio codec IC, supports
s/codec/CODEC/
Yep.
If you agree with something, best just to snip it.
No need to reply at all if you agree with everything.
[...]
+static int wcd9335_power_on_reset(struct wcd9335 *wcd) +{
- struct device *dev = wcd->dev;
- int ret;
- ret = regulator_bulk_enable(WCD9335_MAX_SUPPLY, wcd->supplies);
- if (ret != 0) {
dev_err(dev, "Failed to get supplies: err = %d\n", ret);return ret;- }
- /*
* For WCD9335, it takes about 600us for the Vout_A and* Vout_D to be ready after BUCK_SIDO is powered up.* SYS_RST_N shouldn't be pulled high during this time*/- usleep_range(600, 650);
- gpio_direction_output(wcd->reset_gpio, 0);
- msleep(20);
What's this for? Why can't you just:
This is just making sure that reset line is low before actual the chip is taken out of reset.
If it confused me, it might confuse others.
Best to add a comment to the tune of:
"Toggle reset line off/on to ensure ... "
gpio_direction_output(wcd->reset_gpio, 1);
... and drop the next 2 lines?
[...]
+static const struct mfd_cell wcd9335_devices[] = {
- {
.name = "wcd9335-codec",- },
+};
When are you going to add the other devices?
We are trying to sort of hw setup to test other devices like soundwire controller, will add these once we are in a position to test them.
Ensure that you do, or I'll revert the whole driver as "not an MFD" :)
By the way, one line entries should be placed on one line.
- { .name = "wcd9335-codec" },
[...]
+static int wcd9335_slim_status(struct slim_device *sdev,
enum slim_device_status s)'s' is not a good variable name. Suggest 'status'.
Agreed!
+{
- struct device_node *ifd_np;
What's 'ifd'?
SLIMbus Interface device.
That's a terrible variable name. :)