| Message ID | 20221024165310.246183-4-pierre-louis.bossart@linux.intel.com |
|---|---|
| State | Accepted |
| Commit | 74fe0c4dcb41678543915cb97928c366ac1aaceb |
| Headers | show |
| Series | ASoC: SOF: Intel: HDaudio cleanups | expand |
On Thu, Oct 27, 2022 at 03:58:08PM -0400, Pierre-Louis Bossart wrote: > > > On 10/27/22 14:51, Nathan Chancellor wrote: > > Hi Pierre-Louis, > > > > On Mon, Oct 24, 2022 at 11:52:57AM -0500, Pierre-Louis Bossart wrote: > >> These will be used to add more consistency in the SOF core and > >> drivers. > >> > >> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> > >> Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com> > >> Reviewed-by: Rander Wang <rander.wang@intel.com> > >> Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com> > > > > This change is now in -next as commit 74fe0c4dcb41 ("ASoC: SOF: ops: add > > readb/writeb helpers"), where it breaks the build badly on at least > > ARCH=arm: > > > > $ kmake ARCH=arm CROSS_COMPILE=arm-linux-gnu- allmodconfig sound/soc/sof/ > > ... > > In file included from sound/soc/sof/sof-client.c:16: > > sound/soc/sof/ops.h: In function ‘snd_sof_dsp_writeb’: > > sound/soc/sof/ops.h:309:75: error: macro "writeb" passed 3 arguments, but takes just 2 > > 309 | sof_ops(sdev)->writeb(sdev, sdev->bar[bar] + offset, value); > > | ^ > > In file included from ./include/linux/io.h:13, > > from ./include/linux/irq.h:20, > > from ./include/asm-generic/hardirq.h:17, > > from ./arch/arm/include/asm/hardirq.h:10, > > from ./include/linux/hardirq.h:11, > > from ./include/linux/interrupt.h:11, > > from sound/soc/sof/ops.h:15: > > ./arch/arm/include/asm/io.h:288: note: macro "writeb" defined here > > 288 | #define writeb(v,c) ({ __iowmb(); writeb_relaxed(v,c); }) > > | > > sound/soc/sof/ops.h:309:30: error: statement with no effect [-Werror=unused-value] > > 309 | sof_ops(sdev)->writeb(sdev, sdev->bar[bar] + offset, value); > > | ^ > > sound/soc/sof/ops.h: In function ‘snd_sof_dsp_readb’: > > sound/soc/sof/ops.h:336:74: error: macro "readb" passed 2 arguments, but takes just 1 > > 336 | return sof_ops(sdev)->readb(sdev, sdev->bar[bar] + offset); > > | ^ > > ./arch/arm/include/asm/io.h:284: note: macro "readb" defined here > > 284 | #define readb(c) ({ u8 __v = readb_relaxed(c); __iormb(); __v; }) > > | > > sound/soc/sof/ops.h:336:37: error: returning ‘u8 (*)(struct snd_sof_dev *, void *)’ {aka ‘unsigned char (*)(struct snd_sof_dev *, void *)’} from a function with return type ‘u8’ {aka ‘unsigned char’} makes integer from pointer without a cast [-Werror=int-conversion] > > 336 | return sof_ops(sdev)->readb(sdev, sdev->bar[bar] + offset); > > | ^ > > cc1: all warnings being treated as errors > > ... > > > > I guess the preprocessor gets to these calls first and everything > > explodes from there. Perhaps these should be namespaced somehow? > > Thanks for the report. We've had these patches for a while and always > compile for ARM, and didn't see any issues raised. Meh. > > I don't have a strong appetite for changes in common parts, maybe it's > just simpler to use read8/write8 as callback names to avoid the > conflict, something like the patch attached (compile-tested only). In > hindsight it'd also be more consistent with the use of read64/write64 > that was added earlier in SOF helpers. That diff resolves the build failure for me as well. I will put it into my test matrix and make sure that it does not cause any other build issues. Cheers, Nathan
diff --git a/sound/soc/sof/ops.h b/sound/soc/sof/ops.h index a72f8043be64d..511c798eb1ebb 100644 --- a/sound/soc/sof/ops.h +++ b/sound/soc/sof/ops.h @@ -302,6 +302,15 @@ static inline int snd_sof_debugfs_add_region_item(struct snd_sof_dev *sdev, } /* register IO */ +static inline void snd_sof_dsp_writeb(struct snd_sof_dev *sdev, u32 bar, + u32 offset, u8 value) +{ + if (sof_ops(sdev)->writeb) + sof_ops(sdev)->writeb(sdev, sdev->bar[bar] + offset, value); + else + writeb(value, sdev->bar[bar] + offset); +} + static inline void snd_sof_dsp_write(struct snd_sof_dev *sdev, u32 bar, u32 offset, u32 value) { @@ -320,6 +329,15 @@ static inline void snd_sof_dsp_write64(struct snd_sof_dev *sdev, u32 bar, writeq(value, sdev->bar[bar] + offset); } +static inline u8 snd_sof_dsp_readb(struct snd_sof_dev *sdev, u32 bar, + u32 offset) +{ + if (sof_ops(sdev)->readb) + return sof_ops(sdev)->readb(sdev, sdev->bar[bar] + offset); + else + return readb(sdev->bar[bar] + offset); +} + static inline u32 snd_sof_dsp_read(struct snd_sof_dev *sdev, u32 bar, u32 offset) { diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index 403e81220244a..d3ede97b67590 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -171,6 +171,10 @@ struct snd_sof_dsp_ops { * TODO: consider removing these operations and calling respective * implementations directly */ + void (*writeb)(struct snd_sof_dev *sof_dev, void __iomem *addr, + u8 value); /* optional */ + u8 (*readb)(struct snd_sof_dev *sof_dev, + void __iomem *addr); /* optional */ void (*write)(struct snd_sof_dev *sof_dev, void __iomem *addr, u32 value); /* optional */ u32 (*read)(struct snd_sof_dev *sof_dev,