<html> <head> <meta content="text/html; charset=utf-8" http-equiv="Content-Type"> </head> <body bgcolor="#FFFFFF" text="#000000"> Hi Mark, <div class="moz-cite-prefix">On 29-03-2016 18:31, Mark Brown wrote: </div> <blockquote cite="mid:20160329173158.GB2350@sirena.org.uk" type="cite"> <pre wrap="">On Mon, Mar 28, 2016 at 03:36:10PM +0100, Jose Abreu wrote: </pre> <blockquote type="cite"> <pre wrap="">HDMI audio support was added to the AXS board using an I2S cpu driver and a custom platform driver. The platform driver supports two channels @ 16 bits with rates 32k, 44.1k and 48k. ALSA Simple audio card is used to glue the cpu, platform and codec driver (adv7511). </pre> </blockquote> <pre wrap=""> </pre> <blockquote type="cite"> <pre wrap=""> sound/soc/dwc/Kconfig | 1 + sound/soc/dwc/designware_i2s.c | 385 +++++++++++++++++++++++++++++++++++++++-- </pre> </blockquote> <pre wrap=""> Your changelog appears to describe the writing of a machine driver but this is a large patch adding code to an I2S controller driver. This means I can't review your patch since I can't tell what it is supposed to do. If you've added functionality to this driver you need to send one or more patches each of which adds a single feature to the driver together with a changelog which describes what that feature is. Glancing at the patch I'm not 100% sure that the features you're adding are part of the Synopsis device but I'm not entirely sure. </pre> </blockquote> The major part of this patch is the adding of an ALSA platform driver so that audio comes out of the box in AXS boards but we also added functionalities to the i2s driver and performed one bug fix related with the mask/unmask of interrupts. I will split the patches but they will depend on each other. <blockquote cite="mid:20160329173158.GB2350@sirena.org.uk" type="cite"> <blockquote type="cite"> <pre wrap=""> 2 files changed, 373 insertions(+), 13 deletions(-) diff --git a/sound/soc/dwc/Kconfig b/sound/soc/dwc/Kconfig index d50e085..bc3fae7 100644 --- a/sound/soc/dwc/Kconfig +++ b/sound/soc/dwc/Kconfig @@ -2,6 +2,7 @@ config SND_DESIGNWARE_I2S tristate "Synopsys I2S Device Driver" depends on CLKDEV_LOOKUP select SND_SOC_GENERIC_DMAENGINE_PCM + select SND_SIMPLE_CARD </pre> </blockquote> <pre wrap=""> No, this doesn't make sense - the fact that someone has used a Synopsis I2S controller doesn't mean that they have a system which uses simple-card. If the user wants to use simple-card they need to enable it separately, this is the same pattern we follow for all CPU controller drivers. </pre> </blockquote> I will remove this. <blockquote cite="mid:20160329173158.GB2350@sirena.org.uk" type="cite"> <fieldset class="mimeAttachmentHeader"></fieldset> <pre wrap="">_______________________________________________ dri-devel mailing list <a class="moz-txt-link-abbreviated" href="mailto:dri-devel@lists.freedesktop.org">dri-devel@lists.freedesktop.org</a> <a class="moz-txt-link-freetext" href="https://lists.freedesktop.org/mailman/listinfo/dri-devel">https://lists.freedesktop.org/mailman/listinfo/dri-devel</a> </pre> </blockquote> Best regards, Jose Miguel Abreu </body> </html>