RE: [PATCH v4] ASoC: rt766: add RT766/RT767 SDCA driver

["Shuming [范書銘]" <shumingf@realtek.com>]

> > This patch adds the initial SDCA multi-function codec driver for the RT766
> and RT767.
> >
> > Signed-off-by: Shuming Fan <shumingf@realtek.com>
> > ---
> > +static int rt766_sdca_irq_ctl(struct rt766_sdca_priv *rt766,
> > +                                                       struct
> sdca_function_data *function,
> > +                                                       struct
> snd_soc_component *component,
> > +                                                       struct
> sdca_interrupt_info *info,
> > +                                                       bool
> enabled)
> > +{
> > +     struct device *dev = &rt766->slave->dev;
> > +     struct sdca_interrupt *interrupt;
> > +     struct sdca_control *control;
> > +     struct sdca_entity *entity;
> > +     irq_handler_t handler;
> > +     int i, j, irq, ret;
> > +
> > +     for (i = 0; i < function->num_entities; i++) {
> > +             entity = &function->entities[i];
> > +
> > +             for (j = 0; j < entity->num_controls; j++) {
> > +                     control = &entity->controls[j];
> > +                     irq = control->interrupt_position;
> > +
> > +                     switch (SDCA_CTL_TYPE(entity->type,
> control->sel)) {
> > +                     case SDCA_CTL_TYPE_S(GE, DETECTED_MODE):
> > +                             handler = rt766_sdca_irq_jd_handler;
> > +                             break;
> > +                     case SDCA_CTL_TYPE_S(HIDE,
> HIDTX_CURRENTOWNER):
> > +                             handler = rt766_sdca_irq_btn_handler;
> > +                             break;
> > +                     default:
> > +                             continue;
> > +                     }
> > +
> > +                     interrupt = &info->irqs[irq];
> > +
> > +                     if (enabled) {
> > +                             ret = sdca_irq_data_populate(dev,
> rt766->regmap, component,
> > +
> function, entity, control,
> > +
> interrupt);
> > +                             if (ret)
> > +                                     return ret;
> > +
> > +                             interrupt->priv = rt766;
> > +                             ret = sdca_irq_request(dev, info, irq,
> interrupt->name,
> > +
> handler, interrupt);
> > +                             if (ret) {
> > +                                     dev_err(dev, "failed to
> request irq %s: %d\n",
> > +                                             interrupt->name,
> ret);
> > +                                     sdca_irq_cleanup_late(dev,
> function, info);
> > +                                     return ret;
> > +                             }
> > +                             dev_dbg(dev, "Requesting IRQ %d
> InterruptName=%s\n", irq, interrupt->name);
> > +                     } else {
> > +                             sdca_irq_free(dev, info, irq,
> interrupt->name, interrupt);
> > +                             dev_dbg(dev, "Freeing IRQ %d\n", irq);
> 
> You can still leak the name here, if the machine driver was to
> call:
> 
> rt766_sdca_set_jack_detect(component, jack, data);
> rt766_sdca_set_jack_detect(component, NULL, NULL);
> rt766_sdca_set_jack_detect(component, jack, data);
> 
> I mean if we are happy that will never happen I guess its fine, but given we
> allow the machine driver to clear the IRQ seems wise to allow it to re-allocate
> it too.

Will fix, thanks for pointing it out.

> Thanks,
> Charles