On 05/17/2018 11:22 AM, Marco Felsch wrote:
The ssm2305 is a simple Class-D audio amplifier. A application can turn on/off the device by a gpio. It's also possible to hardwire the shutdown pin.
Tested on a i.MX6 based custom board.
Signed-off-by: Marco Felsch m.felsch@pengutronix.de
Hi,
Thanks for the patch. Looks mostly good, some comments inline.
.../devicetree/bindings/sound/adi,ssm2305.txt | 15 +++ sound/soc/codecs/Kconfig | 7 ++ sound/soc/codecs/Makefile | 2 + sound/soc/codecs/ssm2305.c | 105 ++++++++++++++++++ 4 files changed, 129 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/adi,ssm2305.txt create mode 100644 sound/soc/codecs/ssm2305.c
diff --git a/Documentation/devicetree/bindings/sound/adi,ssm2305.txt b/Documentation/devicetree/bindings/sound/adi,ssm2305.txt new file mode 100644 index 000000000000..fc4c99225538 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/adi,ssm2305.txt @@ -0,0 +1,15 @@ +Analog Devices SSM2305 Speaker Amplifier +========================================
+Required properties:
- compatible : "adi,ssm2305"
+Optional properties:
- ssm2305,shutdown-gpio : the gpio connected to the shutdown pin
I think policy is to use only the -gpios suffix for new bindings.
+Example:
+ssm2305: analog-amplifier {
- compatible = "adi,ssm2305";
- ssm2305,shutdown-gpio = <&gpio3 20 GPIO_ACTIVE_LOW>;
+};
[...]
+static int ssm2305_probe(struct platform_device *pdev) +{
- struct device *dev = &pdev->dev;
- struct ssm2305 *priv;
- int err;
- /* Allocate the private data */
- priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
- if (!priv)
return -ENOMEM;- platform_set_drvdata(pdev, priv);
- /* Shutdown gpio is optional */
If it is really optional you should use gpiod_get_optional. But I wonder what is the point of the driver if the GPIO is not present?
- priv->gpiod_shutdown = devm_gpiod_get(dev, "ssm2305,shutdown",
GPIOD_OUT_LOW);- if (IS_ERR(priv->gpiod_shutdown)) {
err = PTR_ERR(priv->gpiod_shutdown);if (err != -EPROBE_DEFER)dev_warn(dev, "Failed to get 'shutdown' gpio: %d\n",err);
Should err be returned here?
- }
- dev_info(dev, "probed\n");
That's a bit too much noise, imagine every driver did this.
- return devm_snd_soc_register_component(dev, &ssm2305_component_driver,
NULL, 0);+}