At Tue, 9 Jun 2015 15:36:51 +0530, Vinod Koul wrote:
On Mon, Jun 08, 2015 at 05:40:46PM +0200, Takashi Iwai wrote:
At Mon, 8 Jun 2015 21:00:14 +0530, Vinod Koul wrote:
On Mon, Jun 08, 2015 at 03:38:22PM +0530, Vinod Koul wrote:
+#define snd_hdac_ext_bus_ppcap_updateb(dev, reg, mask, val) \
- snd_hdac_ext_bus_ppcap_writeb(dev, reg, \
(snd_hdac_ext_bus_ppcap_readb(dev, reg) & \~(mask)) | (val))It's not necessarily good to wrap all with such macros. For azx_write*(), I kept them as is for reducing the amount of useless code rewrites. But for new codes, I don't think it's always worth...
Actually while updating the patch for ext I was wondering about this too.
So we cna remove these and use snd_hdac_chip_writel/w/b here
As Jeeja pointed we can't use snd_hdac_chip_writel as we need to use a different base. So we cna move this to use plain writel only
Any other ideas?
I don't think you need to access via io_ops redirection as these are SKL specific registers. Use plain readl()/writel() and keep things as simple as possible.
(And better to avoid w and b variants.)
Okay I have removed all these macros but ended up defining one generic update macros (most of the places we are doing read and write, so better to use update macro
/* update register macro */ #define snd_hdac_updatel(addr, reg, mask, val) \ writel(addr, reg, (readl(addr, reg) & ~(mask)) | (val))
yes its updatel to signify that it uses writel and readl
Let me know if you are fine with this approach
This looks good to me.
thanks,
Takashi