Why not put this under arch/arm?
Is there an appropriate place for loadable device drivers under arch/arm? I don't know of one.
+static inline void set_gpio_bit(unsigned n, void __iomem *reg) +{ + writel(readl(reg) | bit(n), reg); +} + +/* + * This function assumes that msm_gpio_dev::lock is held. + */ +static inline void clr_gpio_bit(unsigned n, void __iomem *reg) +{ + writel(readl(reg)& ~bit(n), reg); +} + +/* + * This function assumes that msm_gpio_dev::lock is held. + */ +static inline void +msm_gpio_write(struct msm_gpio_dev *dev, unsigned n, unsigned on) +{ + if (on) + set_gpio_bit(n, dev->regs.out); + else + clr_gpio_bit(n, dev->regs.out); +}wouldn't it be easier to inline a set_to function and just role the set and clr bit functions into it, since they pretty much do the same thing. even better, on arm the code won't require a branch.
I'm not sure I understand you. Can you clarify? set_ and clr_gpio_bit are used in more places than just here, so they can't just be rolled into msm_gpio_write and disappear.
+static int msm_gpio_remove(struct platform_device *dev) +{ + struct msm_gpio_dev *msm_gpio = platform_get_drvdata(dev); + int ret = gpiochip_remove(&msm_gpio->gpio_chip); + + if (ret == 0) + kfree(msm_gpio);hmm, not sure if you really need to check the result here before kfrree() the memory.
I feel that this is important. If any clients are still holding gpio lines, gpiochip_remove will fail. In those circumstances, is it not important that the device not be freed (which would leave clients with stale references) and that the remove call return a proper failure code?
-- Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
