Hi Mark, > > > > +static inline u32 fsl_pwm_readl(struct fsl_pwm_chip *fpc, > > > > + const void __iomem *addr) > > > > +{ > > > > + if (likely(fpc->big_endian)) > > > > + return ioread32be(addr); > > > > + else > > > > + return readl(addr); > > > > +} > > > > It looks a little odd to to have two different accessors here. > > > > Could these not be unified somehow? > > > > How about the following : > > +static inline u32 fsl_pwm_readl(struct fsl_pwm_chip *fpc, > + const void __iomem *addr) > +{ > + u32 val; > + > + if (likely(fpc->big_endian)) > + val = be32_to_cpu(__raw_readl(addr)); > + else > + val = le32_to_cpu(__raw_readl(addr)); > + > + rmb(); > + > + return val; > +} > + > +static inline void fsl_pwm_writel(struct fsl_pwm_chip *fpc, > + u32 val, void __iomem *addr) > +{ > + wmb(); > + > + if (likely(fpc->big_endian)) > + __raw_writel(cpu_to_be32(val), addr); > + else > + __raw_writel(cpu_to_le32(val), addr); > +} > + > > Or, will these be much better ? +++++++++++ +static inline u32 fsl_pwm_readl(struct fsl_pwm_chip *fpc, + const void __iomem *addr) +{ + u32 val; + + if (likely(fpc->big_endian)) + val = be32_to_cpu((__force __be32)__raw_readl(addr)); + else + val = le32_to_cpu((__force __le32)__raw_readl(addr)); + + rmb(); + + return val; +} + +static inline void fsl_pwm_writel(struct fsl_pwm_chip *fpc, + u32 val, void __iomem *addr) +{ + wmb(); + + if (likely(fpc->big_endian)) + __raw_writel((__force u32)cpu_to_be32(val), addr); + else + __raw_writel((__force u32)cpu_to_le32(val), addr); } + ----------- Thanks, -- Xiubo -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html