[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <D4B53566-9F7D-46DA-8BAE-0572B9A8AA7A@cirrus.com>
Date: Tue, 15 Aug 2023 15:56:14 +0000
From: James Ogletree <James.Ogletree@...rus.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
CC: Dmitry Torokhov <dmitry.torokhov@...il.com>,
Fred Treven <Fred.Treven@...rus.com>,
Ben Bright <Ben.Bright@...rus.com>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>, Lee Jones <lee@...nel.org>,
Jeff LaBundy <jeff@...undy.com>, Joel Stanley <joel@....id.au>,
Arnd Bergmann <arnd@...db.de>, Jacky Bai <ping.bai@....com>,
Jean Delvare <jdelvare@...e.de>,
Eddie James <eajames@...ux.ibm.com>,
Markus Schneider-Pargmann <msp@...libre.com>,
ChiYuan Huang <cy_huang@...htek.com>,
Randy Dunlap <rdunlap@...radead.org>,
Wolfram Sang <wsa+renesas@...g-engineering.com>,
"patches@...rus.com" <patches@...rus.com>,
"linux-input@...r.kernel.org" <linux-input@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 2/2] Input: cs40l50 - Initial support for Cirrus Logic
CS40L50
> On Aug 10, 2023, at 1:17 AM, Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org> wrote:
>
> On 09/08/2023 21:10, James Ogletree wrote:
>
>> +
>> +static int cs40l50_cs_dsp_init(struct cs40l50_private *cs40l50)
>> +{
>> + cs40l50->dsp.num = 1;
>> + cs40l50->dsp.type = WMFW_HALO;
>> + cs40l50->dsp.dev = cs40l50->dev;
>> + cs40l50->dsp.regmap = cs40l50->regmap;
>> + cs40l50->dsp.base = CS40L50_DSP1_CORE_BASE;
>> + cs40l50->dsp.base_sysinfo = CS40L50_DSP1_SYS_INFO_ID;
>> + cs40l50->dsp.mem = cs40l50_dsp_regions;
>> + cs40l50->dsp.num_mems = ARRAY_SIZE(cs40l50_dsp_regions);
>> + cs40l50->dsp.lock_regions = 0xFFFFFFFF;
>> + cs40l50->dsp.no_core_startstop = true;
>> + cs40l50->dsp.client_ops = &cs40l50_cs_dsp_client_ops;
>> +
>> + return cs_dsp_halo_init(&cs40l50->dsp);
>> +}
>> +
>> +int cs40l50_probe(struct cs40l50_private *cs40l50)
>> +{
>> + int error, i, irq;
>> + u32 val;
>> +
>> + mutex_init(&cs40l50->lock);
>> +
>> + error = devm_regulator_bulk_get(cs40l50->dev, ARRAY_SIZE(cs40l50_supplies),
>> + cs40l50_supplies);
>> + if (error)
>> + return dev_err_probe(cs40l50->dev, error, "Failed to request supplies\n");
>> +
>> + error = regulator_bulk_enable(ARRAY_SIZE(cs40l50_supplies), cs40l50_supplies);
>> + if (error)
>> + return dev_err_probe(cs40l50->dev, error, "Failed to enable supplies\n");
>> +
>> + cs40l50->reset_gpio = devm_gpiod_get_optional(cs40l50->dev, "reset", GPIOD_OUT_HIGH);
>
> None of the lines above or below seem to be wrapped according to Linux
> coding style (80).
This patch abides by the 100-column line length limit which checkpatch.pl enforces.
However, I can see how some of the lines might be less jarring to the eyes if wrapped.
That will be addressed in V4.
>
>> + if (IS_ERR(cs40l50->reset_gpio)) {
>> + error = PTR_ERR(cs40l50->reset_gpio);
>> + goto err;
>
> Why do you disable IRQ now?
>
> I asked to test this. Your entire cleanup path is:
> 1. not tested.
> 2. buggy.
> 3. done differently than Linux style. Use proper cleanup calls and
> multiple goto labels.
Disabling IRQ there is a mistake. The probe error path will be tightened up and made
to utilize multiple goto labels instead in V4.
Regards,
James Ogletree
Powered by blists - more mailing lists