On Tue, Apr 25, 2023 at 05:59:52PM +0200, Paweł Anikiel wrote:
> > > +config SND_SOC_CHV3
> > > + tristate "SoC Audio support for Chameleon v3"
> > > + select SND_SOC_SSM2602
> > > + select SND_SOC_SSM2602_I2C
> > > + help
> > > + Say Y if you want to add audio support for the Chameleon v3.
> > It woudl be better to have a separate selectable symbol for each drier.
> I'm not sure about this. If I disable just one driver, the entire card
> fails to probe (even if some audio device doesn't need that driver).
> Does it then make sense to be able to deselect some drivers? Please
> correct me if I'm misunderstanding.
Consider what happens if someone for example reuses the I2S controller
in a different board.
> Having said that, I did try to remove that logic and simply delay
> hw_pointer by one frame, and it appears to work (the playback seems
> fine and without glitches). However, I'm worried about calling
> snd_pcm_period_elapsed() and then reporting that the hw_pointer hasn't
> actually reached the end of the period. Is that ok to do?
It should be fine, things should be working off the hw_pointer.
> > > + reg = readl(i2s->iobase_irq + I2S_IRQ_CLR);
> > > + if (!reg)
> > > + return IRQ_NONE;
> > > + if (reg & I2S_IRQ_RX_BIT)
> > > +
> > > + if (reg & I2S_IRQ_TX_BIT) {
> > > + writel(reg, i2s->iobase_irq + I2S_IRQ_CLR);
> > > +
> > > + return IRQ_HANDLED;
> > > +}
> > Really we should only ack things that were handled here and report
> > appropriately, that's defensive against bugs causing interrupts to
> > scream and shared interrupts.
> What do you mean by handled? Should I check the hardware pointer and
> check if a period really has elapsed?
The driver is checking for specific bits in the status register but
blindly acknowledging all bits that are set, and reporting IRQ_HANDLED
even if none are set.
> > why is it not being added as a CODEC?
> Do you mean I should put it in sound/soc/codecs/?
Yes.
> Also, I used the name of the HDMI receiver chip (IT68051), but really
> this goes through some extra processing in an FPGA, so the result has
> little in common with the chip itself. Do you have any advice on how
> it should be named?
If it's genuinely unrelated to the capabilities of the actual chip then
just putting a standalone driver in the platform directory is fine, but
the code should be clear about that otherwise it looks like the code for
the device could be shared.
Attachment:
signature.asc
Description: PGP signature
