> On Aug 10, 2023, at 1:17 AM, Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> 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