| Message ID | 1485435631-32642-1-git-send-email-baoyou.xie@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3,1/3] dt: bindings: add documentation for zx2967 family i2c controller | expand |
Only a couple of comments below. Otherwise, the patch looks fine to me. On Thu, Jan 26, 2017 at 09:00:31PM +0800, Baoyou Xie wrote: <snip> > +static int zx2967_i2c_flush_fifos(struct zx2967_i2c_info *zx_i2c) > +{ > + u32 val; > + u32 offset; > + > + if (zx_i2c->msg_rd) { > + offset = REG_RDCONF; > + val = I2C_RFIFO_RESET; > + } else { > + offset = REG_WRCONF; > + val = I2C_WFIFO_RESET; > + } > + > + val |= zx2967_i2c_readl(zx_i2c, offset); > + zx2967_i2c_writel(zx_i2c, val, offset); > + > + return 0; Since the function will never return other values than 0, should we simply define the return type as void? > +} <snip> > +static int zx2967_i2c_probe(struct platform_device *pdev) > +{ > + struct zx2967_i2c_info *zx_i2c; > + void __iomem *reg_base; > + struct resource *res; > + struct clk *clk; > + int ret; > + > + zx_i2c = devm_kzalloc(&pdev->dev, sizeof(*zx_i2c), GFP_KERNEL); > + if (!zx_i2c) > + return -ENOMEM; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + reg_base = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(reg_base)) > + return PTR_ERR(reg_base); > + > + clk = devm_clk_get(&pdev->dev, NULL); > + if (IS_ERR(clk)) { > + dev_err(&pdev->dev, "missing controller clock"); > + return PTR_ERR(clk); > + } > + > + ret = clk_prepare_enable(clk); > + if (ret) { > + dev_err(&pdev->dev, "failed to enable i2c_clk\n"); > + return ret; > + } > + > + ret = platform_get_irq(pdev, 0); > + if (ret < 0) > + return ret; > + zx_i2c->irq = ret; > + > + ret = device_property_read_u32(&pdev->dev, "clock-frequency", > + &zx_i2c->clk_freq); > + if (ret) { > + dev_err(&pdev->dev, "missing clock-frequency"); > + return ret; > + } > + > + zx_i2c->reg_base = reg_base; > + zx_i2c->clk = clk; > + zx_i2c->dev = &pdev->dev; > + > + zx_i2c->pin_ctrl = devm_pinctrl_get_select_default(&pdev->dev); > + if (IS_ERR(zx_i2c->pin_ctrl)) { > + if (PTR_ERR(zx_i2c->pin_ctrl) == -EPROBE_DEFER) > + return -EPROBE_DEFER; > + dev_info(&pdev->dev, "no pinctrl handle\n"); > + } You do not need this boilerplate code for request "default" pin state, since device core will automatically issue the call. See section "Pin control requests from drivers" in Documentation/pinctrl.txt for more info. Shawn -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/i2c/i2c-zx2967.txt b/Documentation/devicetree/bindings/i2c/i2c-zx2967.txt new file mode 100644 index 0000000..a528374 --- /dev/null +++ b/Documentation/devicetree/bindings/i2c/i2c-zx2967.txt @@ -0,0 +1,24 @@ +ZTE zx2967 I2C controller + +Required properties: + - compatible: must be "zte,zx296718-i2c" + - reg: physical address and length of the device registers + - interrupts: a single interrupt specifier + - clocks: clock for the device + - #address-cells: should be <1> + - #size-cells: should be <0> + - clock-frequency: the desired I2C bus clock frequency. + +Examples: + + i2c@112000 { + compatible = "zte,zx296718-i2c"; + reg = <0x00112000 0x1000>; + interrupts = <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&osc24m>; + #address-cells = <1> + #size-cells = <0>; + clock-frequency = <1600000>; + pinctrl-names = "default"; + pinctrl-0 = <&i2c_global_pin>; + };