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

["Shuming [范書銘]" <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);
> > +                                     return ret;
> > +                             }
> > +                             dev_dbg(dev, "Requesting IRQ %d
> InterruptName=%s\n", irq, interrupt->name);
> > +                     } else {
> > +                             sdca_irq_free(dev, info, irq,
> > + interrupt->name, interrupt);
> 
> You need to also free the IRQ name allocated by
> sdca_irq_data_populate() here or it will leak if the jack is re-enabled.

Thanks for review.
I will call sdca_irq_cleanup() to free the IRQ.

> > +static void rt766_sdca_remove(struct snd_soc_component *component) {
> > +     struct rt766_sdca_priv *rt766  =
> > +snd_soc_component_get_drvdata(component);
> > +
> > +     sdca_irq_cleanup(component->dev, rt766->uaj_func_data,
> rt766->irq_info);
> > +     sdca_irq_cleanup(component->dev, rt766->hid_func_data,
> > +rt766->irq_info); }
> 
> These should be sdca_irq_cleanup_late(), to ensure they free the names as
> well. Although, worth noting this only does anything if
> rt766_sdca_set_jack_detect() didn't already clean everything up, not sure if
> that is guaranteed or not.

Sure, will use sdca_irq_cleanup_late().

> Thanks,
> Charles