All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
@ 2015-09-05  8:21 ` Hans de Goede
  0 siblings, 0 replies; 44+ messages in thread
From: Hans de Goede @ 2015-09-05  8:21 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Hans de Goede

The Point of View protab2-ips9 is a tablet with a 9" ips 1024x768 lcd screen,
microsd slot, headphones, mini hdmi, mini usb b and power barrel connectors.

It uses a rtl8188cus usb wifi chip and a RDA 5875Y bluetooth chip attached
to uart2. It has a bma250 accelerometer attached to i2c1 addr 0x18.

It has a pixcir,pixcir_tangoc compatible touchscreen attached to i2c2 addr
0x5c. This is not enabled in this dts, because this variant of the
pixcir_tangoc has separate wakeup and enable pins both of which need
to be driven low before the touchscreen will work. Before we can enable
this the pixcir driver and devicetree-bindings need to be extended to
support these pins.

Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 arch/arm/boot/dts/Makefile                       |   3 +-
 arch/arm/boot/dts/sun4i-a10-pov-protab2-ips9.dts | 209 +++++++++++++++++++++++
 2 files changed, 211 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/boot/dts/sun4i-a10-pov-protab2-ips9.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index e981fd6..c08883c 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -561,7 +561,8 @@ dtb-$(CONFIG_MACH_SUN4I) += \
 	sun4i-a10-mk802.dtb \
 	sun4i-a10-mk802ii.dtb \
 	sun4i-a10-olinuxino-lime.dtb \
-	sun4i-a10-pcduino.dtb
+	sun4i-a10-pcduino.dtb \
+	sun4i-a10-pov-protab2-ips9.dtb
 dtb-$(CONFIG_MACH_SUN5I) += \
 	sun5i-a10s-auxtek-t003.dtb \
 	sun5i-a10s-auxtek-t004.dtb \
diff --git a/arch/arm/boot/dts/sun4i-a10-pov-protab2-ips9.dts b/arch/arm/boot/dts/sun4i-a10-pov-protab2-ips9.dts
new file mode 100644
index 0000000..223515e
--- /dev/null
+++ b/arch/arm/boot/dts/sun4i-a10-pov-protab2-ips9.dts
@@ -0,0 +1,209 @@
+/*
+ * Copyright 2015 Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file is free software; you can redistribute it and/or
+ *     modify it under the terms of the GNU General Public License as
+ *     published by the Free Software Foundation; either version 2 of the
+ *     License, or (at your option) any later version.
+ *
+ *     This file is distributed in the hope that it will be useful,
+ *     but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *     GNU General Public License for more details.
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use,
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+#include "sun4i-a10.dtsi"
+#include "sunxi-common-regulators.dtsi"
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/pinctrl/sun4i-a10.h>
+
+/ {
+	model = "Point of View Protab2-IPS9";
+	compatible = "pov,protab2-ips9", "allwinner,sun4i-a10";
+
+	aliases {
+		serial0 = &uart0;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+};
+
+&cpu0 {
+	cpu-supply = <&reg_dcdc2>;
+};
+
+&ehci0 {
+	status = "okay";
+};
+
+&i2c0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c0_pins_a>;
+	status = "okay";
+
+	axp209: pmic@34 {
+		reg = <0x34>;
+		interrupts = <0>;
+	};
+};
+
+#include "axp209.dtsi"
+
+&i2c1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c1_pins_a>;
+	status = "okay";
+};
+
+&i2c2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c2_pins_a>;
+	status = "okay";
+};
+
+&lradc {
+	vref-supply = <&reg_ldo2>;
+	status = "okay";
+
+	button@400 {
+		label = "Volume Up";
+		linux,code = <KEY_VOLUMEUP>;
+		channel = <0>;
+		voltage = <400000>;
+	};
+
+	button@800 {
+		label = "Volume Down";
+		linux,code = <KEY_VOLUMEDOWN>;
+		channel = <0>;
+		voltage = <800000>;
+	};
+};
+
+&mmc0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&mmc0_pins_a>, <&mmc0_cd_pin_reference_design>;
+	vmmc-supply = <&reg_vcc3v3>;
+	bus-width = <4>;
+	cd-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>; /* PH1 */
+	cd-inverted;
+	status = "okay";
+};
+
+&otg_sram {
+	status = "okay";
+};
+
+&pio {
+	usb0_id_detect_pin: usb0_id_detect_pin@0 {
+		allwinner,pins = "PH4";
+		allwinner,function = "gpio_in";
+		allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+		allwinner,pull = <SUN4I_PINCTRL_PULL_UP>;
+	};
+
+	usb0_vbus_detect_pin: usb0_vbus_detect_pin@0 {
+		allwinner,pins = "PH5";
+		allwinner,function = "gpio_in";
+		allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+		allwinner,pull = <SUN4I_PINCTRL_PULL_DOWN>;
+	};
+};
+
+&reg_dcdc2 {
+	regulator-always-on;
+	regulator-min-microvolt = <1000000>;
+	regulator-max-microvolt = <1400000>;
+	regulator-name = "vdd-cpu";
+};
+
+&reg_dcdc3 {
+	regulator-always-on;
+	regulator-min-microvolt = <1250000>;
+	regulator-max-microvolt = <1250000>;
+	regulator-name = "vdd-int-dll";
+};
+
+&reg_ldo1 {
+	regulator-name = "vdd-rtc";
+};
+
+&reg_ldo2 {
+	regulator-always-on;
+	regulator-min-microvolt = <3000000>;
+	regulator-max-microvolt = <3000000>;
+	regulator-name = "avcc";
+};
+
+&reg_ldo3 {
+	/*
+	 * We need to always power the camera sensor, otherwhise all access
+	 * to i2c1 is blocked.
+	 */
+	regulator-always-on;
+	regulator-min-microvolt = <2800000>;
+	regulator-max-microvolt = <2800000>;
+	regulator-name = "vdd-csi";
+};
+
+&reg_usb0_vbus {
+	status = "okay";
+};
+
+&reg_usb1_vbus {
+	status = "okay";
+};
+
+&uart0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart0_pins_a>;
+	status = "okay";
+};
+
+&usb_otg {
+	dr_mode = "otg";
+	status = "okay";
+};
+
+&usbphy {
+	pinctrl-names = "default";
+	pinctrl-0 = <&usb0_id_detect_pin>, <&usb0_vbus_detect_pin>;
+	usb0_id_det-gpio = <&pio 7 4 GPIO_ACTIVE_HIGH>; /* PH4 */
+	usb0_vbus_det-gpio = <&pio 7 5 GPIO_ACTIVE_HIGH>; /* PH5 */
+	usb0_vbus-supply = <&reg_usb0_vbus>;
+	usb1_vbus-supply = <&reg_usb1_vbus>;
+	status = "okay";
+};
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
@ 2015-09-05  8:21 ` Hans de Goede
  0 siblings, 0 replies; 44+ messages in thread
From: Hans de Goede @ 2015-09-05  8:21 UTC (permalink / raw)
  To: linux-arm-kernel

The Point of View protab2-ips9 is a tablet with a 9" ips 1024x768 lcd screen,
microsd slot, headphones, mini hdmi, mini usb b and power barrel connectors.

It uses a rtl8188cus usb wifi chip and a RDA 5875Y bluetooth chip attached
to uart2. It has a bma250 accelerometer attached to i2c1 addr 0x18.

It has a pixcir,pixcir_tangoc compatible touchscreen attached to i2c2 addr
0x5c. This is not enabled in this dts, because this variant of the
pixcir_tangoc has separate wakeup and enable pins both of which need
to be driven low before the touchscreen will work. Before we can enable
this the pixcir driver and devicetree-bindings need to be extended to
support these pins.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 arch/arm/boot/dts/Makefile                       |   3 +-
 arch/arm/boot/dts/sun4i-a10-pov-protab2-ips9.dts | 209 +++++++++++++++++++++++
 2 files changed, 211 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/boot/dts/sun4i-a10-pov-protab2-ips9.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index e981fd6..c08883c 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -561,7 +561,8 @@ dtb-$(CONFIG_MACH_SUN4I) += \
 	sun4i-a10-mk802.dtb \
 	sun4i-a10-mk802ii.dtb \
 	sun4i-a10-olinuxino-lime.dtb \
-	sun4i-a10-pcduino.dtb
+	sun4i-a10-pcduino.dtb \
+	sun4i-a10-pov-protab2-ips9.dtb
 dtb-$(CONFIG_MACH_SUN5I) += \
 	sun5i-a10s-auxtek-t003.dtb \
 	sun5i-a10s-auxtek-t004.dtb \
diff --git a/arch/arm/boot/dts/sun4i-a10-pov-protab2-ips9.dts b/arch/arm/boot/dts/sun4i-a10-pov-protab2-ips9.dts
new file mode 100644
index 0000000..223515e
--- /dev/null
+++ b/arch/arm/boot/dts/sun4i-a10-pov-protab2-ips9.dts
@@ -0,0 +1,209 @@
+/*
+ * Copyright 2015 Hans de Goede <hdegoede@redhat.com>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file is free software; you can redistribute it and/or
+ *     modify it under the terms of the GNU General Public License as
+ *     published by the Free Software Foundation; either version 2 of the
+ *     License, or (at your option) any later version.
+ *
+ *     This file is distributed in the hope that it will be useful,
+ *     but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *     GNU General Public License for more details.
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use,
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+#include "sun4i-a10.dtsi"
+#include "sunxi-common-regulators.dtsi"
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/pinctrl/sun4i-a10.h>
+
+/ {
+	model = "Point of View Protab2-IPS9";
+	compatible = "pov,protab2-ips9", "allwinner,sun4i-a10";
+
+	aliases {
+		serial0 = &uart0;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+};
+
+&cpu0 {
+	cpu-supply = <&reg_dcdc2>;
+};
+
+&ehci0 {
+	status = "okay";
+};
+
+&i2c0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c0_pins_a>;
+	status = "okay";
+
+	axp209: pmic at 34 {
+		reg = <0x34>;
+		interrupts = <0>;
+	};
+};
+
+#include "axp209.dtsi"
+
+&i2c1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c1_pins_a>;
+	status = "okay";
+};
+
+&i2c2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c2_pins_a>;
+	status = "okay";
+};
+
+&lradc {
+	vref-supply = <&reg_ldo2>;
+	status = "okay";
+
+	button at 400 {
+		label = "Volume Up";
+		linux,code = <KEY_VOLUMEUP>;
+		channel = <0>;
+		voltage = <400000>;
+	};
+
+	button at 800 {
+		label = "Volume Down";
+		linux,code = <KEY_VOLUMEDOWN>;
+		channel = <0>;
+		voltage = <800000>;
+	};
+};
+
+&mmc0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&mmc0_pins_a>, <&mmc0_cd_pin_reference_design>;
+	vmmc-supply = <&reg_vcc3v3>;
+	bus-width = <4>;
+	cd-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>; /* PH1 */
+	cd-inverted;
+	status = "okay";
+};
+
+&otg_sram {
+	status = "okay";
+};
+
+&pio {
+	usb0_id_detect_pin: usb0_id_detect_pin at 0 {
+		allwinner,pins = "PH4";
+		allwinner,function = "gpio_in";
+		allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+		allwinner,pull = <SUN4I_PINCTRL_PULL_UP>;
+	};
+
+	usb0_vbus_detect_pin: usb0_vbus_detect_pin at 0 {
+		allwinner,pins = "PH5";
+		allwinner,function = "gpio_in";
+		allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+		allwinner,pull = <SUN4I_PINCTRL_PULL_DOWN>;
+	};
+};
+
+&reg_dcdc2 {
+	regulator-always-on;
+	regulator-min-microvolt = <1000000>;
+	regulator-max-microvolt = <1400000>;
+	regulator-name = "vdd-cpu";
+};
+
+&reg_dcdc3 {
+	regulator-always-on;
+	regulator-min-microvolt = <1250000>;
+	regulator-max-microvolt = <1250000>;
+	regulator-name = "vdd-int-dll";
+};
+
+&reg_ldo1 {
+	regulator-name = "vdd-rtc";
+};
+
+&reg_ldo2 {
+	regulator-always-on;
+	regulator-min-microvolt = <3000000>;
+	regulator-max-microvolt = <3000000>;
+	regulator-name = "avcc";
+};
+
+&reg_ldo3 {
+	/*
+	 * We need to always power the camera sensor, otherwhise all access
+	 * to i2c1 is blocked.
+	 */
+	regulator-always-on;
+	regulator-min-microvolt = <2800000>;
+	regulator-max-microvolt = <2800000>;
+	regulator-name = "vdd-csi";
+};
+
+&reg_usb0_vbus {
+	status = "okay";
+};
+
+&reg_usb1_vbus {
+	status = "okay";
+};
+
+&uart0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart0_pins_a>;
+	status = "okay";
+};
+
+&usb_otg {
+	dr_mode = "otg";
+	status = "okay";
+};
+
+&usbphy {
+	pinctrl-names = "default";
+	pinctrl-0 = <&usb0_id_detect_pin>, <&usb0_vbus_detect_pin>;
+	usb0_id_det-gpio = <&pio 7 4 GPIO_ACTIVE_HIGH>; /* PH4 */
+	usb0_vbus_det-gpio = <&pio 7 5 GPIO_ACTIVE_HIGH>; /* PH5 */
+	usb0_vbus-supply = <&reg_usb0_vbus>;
+	usb1_vbus-supply = <&reg_usb1_vbus>;
+	status = "okay";
+};
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
  2015-09-05  8:21 ` Hans de Goede
@ 2015-09-06 16:30     ` Maxime Ripard
  -1 siblings, 0 replies; 44+ messages in thread
From: Maxime Ripard @ 2015-09-06 16:30 UTC (permalink / raw)
  To: Hans de Goede
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw

[-- Attachment #1: Type: text/plain, Size: 7175 bytes --]

On Sat, Sep 05, 2015 at 10:21:59AM +0200, Hans de Goede wrote:
> The Point of View protab2-ips9 is a tablet with a 9" ips 1024x768 lcd screen,
> microsd slot, headphones, mini hdmi, mini usb b and power barrel connectors.
> 
> It uses a rtl8188cus usb wifi chip and a RDA 5875Y bluetooth chip attached
> to uart2. It has a bma250 accelerometer attached to i2c1 addr 0x18.
> 
> It has a pixcir,pixcir_tangoc compatible touchscreen attached to i2c2 addr
> 0x5c. This is not enabled in this dts, because this variant of the
> pixcir_tangoc has separate wakeup and enable pins both of which need
> to be driven low before the touchscreen will work. Before we can enable
> this the pixcir driver and devicetree-bindings need to be extended to
> support these pins.
> 
> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  arch/arm/boot/dts/Makefile                       |   3 +-
>  arch/arm/boot/dts/sun4i-a10-pov-protab2-ips9.dts | 209 +++++++++++++++++++++++
>  2 files changed, 211 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/boot/dts/sun4i-a10-pov-protab2-ips9.dts
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index e981fd6..c08883c 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -561,7 +561,8 @@ dtb-$(CONFIG_MACH_SUN4I) += \
>  	sun4i-a10-mk802.dtb \
>  	sun4i-a10-mk802ii.dtb \
>  	sun4i-a10-olinuxino-lime.dtb \
> -	sun4i-a10-pcduino.dtb
> +	sun4i-a10-pcduino.dtb \
> +	sun4i-a10-pov-protab2-ips9.dtb
>  dtb-$(CONFIG_MACH_SUN5I) += \
>  	sun5i-a10s-auxtek-t003.dtb \
>  	sun5i-a10s-auxtek-t004.dtb \
> diff --git a/arch/arm/boot/dts/sun4i-a10-pov-protab2-ips9.dts b/arch/arm/boot/dts/sun4i-a10-pov-protab2-ips9.dts
> new file mode 100644
> index 0000000..223515e
> --- /dev/null
> +++ b/arch/arm/boot/dts/sun4i-a10-pov-protab2-ips9.dts
> @@ -0,0 +1,209 @@
> +/*
> + * Copyright 2015 Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + *  a) This file is free software; you can redistribute it and/or
> + *     modify it under the terms of the GNU General Public License as
> + *     published by the Free Software Foundation; either version 2 of the
> + *     License, or (at your option) any later version.
> + *
> + *     This file is distributed in the hope that it will be useful,
> + *     but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *     GNU General Public License for more details.
> + *
> + * Or, alternatively,
> + *
> + *  b) Permission is hereby granted, free of charge, to any person
> + *     obtaining a copy of this software and associated documentation
> + *     files (the "Software"), to deal in the Software without
> + *     restriction, including without limitation the rights to use,
> + *     copy, modify, merge, publish, distribute, sublicense, and/or
> + *     sell copies of the Software, and to permit persons to whom the
> + *     Software is furnished to do so, subject to the following
> + *     conditions:
> + *
> + *     The above copyright notice and this permission notice shall be
> + *     included in all copies or substantial portions of the Software.
> + *
> + *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> + *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> + *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + *     OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +/dts-v1/;
> +#include "sun4i-a10.dtsi"
> +#include "sunxi-common-regulators.dtsi"
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/pinctrl/sun4i-a10.h>
> +
> +/ {
> +	model = "Point of View Protab2-IPS9";
> +	compatible = "pov,protab2-ips9", "allwinner,sun4i-a10";
> +
> +	aliases {
> +		serial0 = &uart0;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +};
> +
> +&cpu0 {
> +	cpu-supply = <&reg_dcdc2>;
> +};
> +
> +&ehci0 {
> +	status = "okay";
> +};
> +
> +&i2c0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c0_pins_a>;
> +	status = "okay";
> +
> +	axp209: pmic@34 {
> +		reg = <0x34>;
> +		interrupts = <0>;
> +	};
> +};
> +
> +#include "axp209.dtsi"
> +
> +&i2c1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c1_pins_a>;
> +	status = "okay";
> +};
> +
> +&i2c2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c2_pins_a>;
> +	status = "okay";
> +};
> +
> +&lradc {
> +	vref-supply = <&reg_ldo2>;
> +	status = "okay";
> +
> +	button@400 {
> +		label = "Volume Up";
> +		linux,code = <KEY_VOLUMEUP>;
> +		channel = <0>;
> +		voltage = <400000>;
> +	};
> +
> +	button@800 {
> +		label = "Volume Down";
> +		linux,code = <KEY_VOLUMEDOWN>;
> +		channel = <0>;
> +		voltage = <800000>;
> +	};
> +};
> +
> +&mmc0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&mmc0_pins_a>, <&mmc0_cd_pin_reference_design>;
> +	vmmc-supply = <&reg_vcc3v3>;
> +	bus-width = <4>;
> +	cd-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>; /* PH1 */
> +	cd-inverted;
> +	status = "okay";
> +};
> +
> +&otg_sram {
> +	status = "okay";
> +};
> +
> +&pio {
> +	usb0_id_detect_pin: usb0_id_detect_pin@0 {
> +		allwinner,pins = "PH4";
> +		allwinner,function = "gpio_in";
> +		allwinner,drive = <SUN4I_PINCTRL_10_MA>;
> +		allwinner,pull = <SUN4I_PINCTRL_PULL_UP>;
> +	};
> +
> +	usb0_vbus_detect_pin: usb0_vbus_detect_pin@0 {
> +		allwinner,pins = "PH5";
> +		allwinner,function = "gpio_in";
> +		allwinner,drive = <SUN4I_PINCTRL_10_MA>;
> +		allwinner,pull = <SUN4I_PINCTRL_PULL_DOWN>;
> +	};
> +};
> +
> +&reg_dcdc2 {
> +	regulator-always-on;
> +	regulator-min-microvolt = <1000000>;
> +	regulator-max-microvolt = <1400000>;
> +	regulator-name = "vdd-cpu";
> +};
> +
> +&reg_dcdc3 {
> +	regulator-always-on;
> +	regulator-min-microvolt = <1250000>;
> +	regulator-max-microvolt = <1250000>;
> +	regulator-name = "vdd-int-dll";
> +};
> +
> +&reg_ldo1 {
> +	regulator-name = "vdd-rtc";
> +};
> +
> +&reg_ldo2 {
> +	regulator-always-on;
> +	regulator-min-microvolt = <3000000>;
> +	regulator-max-microvolt = <3000000>;
> +	regulator-name = "avcc";
> +};
> +
> +&reg_ldo3 {
> +	/*
> +	 * We need to always power the camera sensor, otherwhise all access
> +	 * to i2c1 is blocked.
> +	 */
> +	regulator-always-on;
> +	regulator-min-microvolt = <2800000>;
> +	regulator-max-microvolt = <2800000>;
> +	regulator-name = "vdd-csi";
> +};

What is connected on i2c1 ? Just the camera sensor? or it has some
other devices there?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
@ 2015-09-06 16:30     ` Maxime Ripard
  0 siblings, 0 replies; 44+ messages in thread
From: Maxime Ripard @ 2015-09-06 16:30 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Sep 05, 2015 at 10:21:59AM +0200, Hans de Goede wrote:
> The Point of View protab2-ips9 is a tablet with a 9" ips 1024x768 lcd screen,
> microsd slot, headphones, mini hdmi, mini usb b and power barrel connectors.
> 
> It uses a rtl8188cus usb wifi chip and a RDA 5875Y bluetooth chip attached
> to uart2. It has a bma250 accelerometer attached to i2c1 addr 0x18.
> 
> It has a pixcir,pixcir_tangoc compatible touchscreen attached to i2c2 addr
> 0x5c. This is not enabled in this dts, because this variant of the
> pixcir_tangoc has separate wakeup and enable pins both of which need
> to be driven low before the touchscreen will work. Before we can enable
> this the pixcir driver and devicetree-bindings need to be extended to
> support these pins.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  arch/arm/boot/dts/Makefile                       |   3 +-
>  arch/arm/boot/dts/sun4i-a10-pov-protab2-ips9.dts | 209 +++++++++++++++++++++++
>  2 files changed, 211 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/boot/dts/sun4i-a10-pov-protab2-ips9.dts
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index e981fd6..c08883c 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -561,7 +561,8 @@ dtb-$(CONFIG_MACH_SUN4I) += \
>  	sun4i-a10-mk802.dtb \
>  	sun4i-a10-mk802ii.dtb \
>  	sun4i-a10-olinuxino-lime.dtb \
> -	sun4i-a10-pcduino.dtb
> +	sun4i-a10-pcduino.dtb \
> +	sun4i-a10-pov-protab2-ips9.dtb
>  dtb-$(CONFIG_MACH_SUN5I) += \
>  	sun5i-a10s-auxtek-t003.dtb \
>  	sun5i-a10s-auxtek-t004.dtb \
> diff --git a/arch/arm/boot/dts/sun4i-a10-pov-protab2-ips9.dts b/arch/arm/boot/dts/sun4i-a10-pov-protab2-ips9.dts
> new file mode 100644
> index 0000000..223515e
> --- /dev/null
> +++ b/arch/arm/boot/dts/sun4i-a10-pov-protab2-ips9.dts
> @@ -0,0 +1,209 @@
> +/*
> + * Copyright 2015 Hans de Goede <hdegoede@redhat.com>
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + *  a) This file is free software; you can redistribute it and/or
> + *     modify it under the terms of the GNU General Public License as
> + *     published by the Free Software Foundation; either version 2 of the
> + *     License, or (at your option) any later version.
> + *
> + *     This file is distributed in the hope that it will be useful,
> + *     but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *     GNU General Public License for more details.
> + *
> + * Or, alternatively,
> + *
> + *  b) Permission is hereby granted, free of charge, to any person
> + *     obtaining a copy of this software and associated documentation
> + *     files (the "Software"), to deal in the Software without
> + *     restriction, including without limitation the rights to use,
> + *     copy, modify, merge, publish, distribute, sublicense, and/or
> + *     sell copies of the Software, and to permit persons to whom the
> + *     Software is furnished to do so, subject to the following
> + *     conditions:
> + *
> + *     The above copyright notice and this permission notice shall be
> + *     included in all copies or substantial portions of the Software.
> + *
> + *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> + *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> + *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + *     OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +/dts-v1/;
> +#include "sun4i-a10.dtsi"
> +#include "sunxi-common-regulators.dtsi"
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/pinctrl/sun4i-a10.h>
> +
> +/ {
> +	model = "Point of View Protab2-IPS9";
> +	compatible = "pov,protab2-ips9", "allwinner,sun4i-a10";
> +
> +	aliases {
> +		serial0 = &uart0;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +};
> +
> +&cpu0 {
> +	cpu-supply = <&reg_dcdc2>;
> +};
> +
> +&ehci0 {
> +	status = "okay";
> +};
> +
> +&i2c0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c0_pins_a>;
> +	status = "okay";
> +
> +	axp209: pmic at 34 {
> +		reg = <0x34>;
> +		interrupts = <0>;
> +	};
> +};
> +
> +#include "axp209.dtsi"
> +
> +&i2c1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c1_pins_a>;
> +	status = "okay";
> +};
> +
> +&i2c2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c2_pins_a>;
> +	status = "okay";
> +};
> +
> +&lradc {
> +	vref-supply = <&reg_ldo2>;
> +	status = "okay";
> +
> +	button at 400 {
> +		label = "Volume Up";
> +		linux,code = <KEY_VOLUMEUP>;
> +		channel = <0>;
> +		voltage = <400000>;
> +	};
> +
> +	button at 800 {
> +		label = "Volume Down";
> +		linux,code = <KEY_VOLUMEDOWN>;
> +		channel = <0>;
> +		voltage = <800000>;
> +	};
> +};
> +
> +&mmc0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&mmc0_pins_a>, <&mmc0_cd_pin_reference_design>;
> +	vmmc-supply = <&reg_vcc3v3>;
> +	bus-width = <4>;
> +	cd-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>; /* PH1 */
> +	cd-inverted;
> +	status = "okay";
> +};
> +
> +&otg_sram {
> +	status = "okay";
> +};
> +
> +&pio {
> +	usb0_id_detect_pin: usb0_id_detect_pin at 0 {
> +		allwinner,pins = "PH4";
> +		allwinner,function = "gpio_in";
> +		allwinner,drive = <SUN4I_PINCTRL_10_MA>;
> +		allwinner,pull = <SUN4I_PINCTRL_PULL_UP>;
> +	};
> +
> +	usb0_vbus_detect_pin: usb0_vbus_detect_pin at 0 {
> +		allwinner,pins = "PH5";
> +		allwinner,function = "gpio_in";
> +		allwinner,drive = <SUN4I_PINCTRL_10_MA>;
> +		allwinner,pull = <SUN4I_PINCTRL_PULL_DOWN>;
> +	};
> +};
> +
> +&reg_dcdc2 {
> +	regulator-always-on;
> +	regulator-min-microvolt = <1000000>;
> +	regulator-max-microvolt = <1400000>;
> +	regulator-name = "vdd-cpu";
> +};
> +
> +&reg_dcdc3 {
> +	regulator-always-on;
> +	regulator-min-microvolt = <1250000>;
> +	regulator-max-microvolt = <1250000>;
> +	regulator-name = "vdd-int-dll";
> +};
> +
> +&reg_ldo1 {
> +	regulator-name = "vdd-rtc";
> +};
> +
> +&reg_ldo2 {
> +	regulator-always-on;
> +	regulator-min-microvolt = <3000000>;
> +	regulator-max-microvolt = <3000000>;
> +	regulator-name = "avcc";
> +};
> +
> +&reg_ldo3 {
> +	/*
> +	 * We need to always power the camera sensor, otherwhise all access
> +	 * to i2c1 is blocked.
> +	 */
> +	regulator-always-on;
> +	regulator-min-microvolt = <2800000>;
> +	regulator-max-microvolt = <2800000>;
> +	regulator-name = "vdd-csi";
> +};

What is connected on i2c1 ? Just the camera sensor? or it has some
other devices there?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150906/1d101747/attachment.sig>

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
  2015-09-06 16:30     ` Maxime Ripard
@ 2015-09-07  7:05       ` Hans de Goede
  -1 siblings, 0 replies; 44+ messages in thread
From: Hans de Goede @ 2015-09-07  7:05 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw

Hi,

On 06-09-15 18:30, Maxime Ripard wrote:
> On Sat, Sep 05, 2015 at 10:21:59AM +0200, Hans de Goede wrote:
>> The Point of View protab2-ips9 is a tablet with a 9" ips 1024x768 lcd screen,
>> microsd slot, headphones, mini hdmi, mini usb b and power barrel connectors.
>>
>> It uses a rtl8188cus usb wifi chip and a RDA 5875Y bluetooth chip attached
>> to uart2. It has a bma250 accelerometer attached to i2c1 addr 0x18.
>>
>> It has a pixcir,pixcir_tangoc compatible touchscreen attached to i2c2 addr
>> 0x5c. This is not enabled in this dts, because this variant of the
>> pixcir_tangoc has separate wakeup and enable pins both of which need
>> to be driven low before the touchscreen will work. Before we can enable
>> this the pixcir driver and devicetree-bindings need to be extended to
>> support these pins.
>>
>> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> ---
>>   arch/arm/boot/dts/Makefile                       |   3 +-
>>   arch/arm/boot/dts/sun4i-a10-pov-protab2-ips9.dts | 209 +++++++++++++++++++++++
>>   2 files changed, 211 insertions(+), 1 deletion(-)
>>   create mode 100644 arch/arm/boot/dts/sun4i-a10-pov-protab2-ips9.dts
>>
>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>> index e981fd6..c08883c 100644
>> --- a/arch/arm/boot/dts/Makefile
>> +++ b/arch/arm/boot/dts/Makefile
>> @@ -561,7 +561,8 @@ dtb-$(CONFIG_MACH_SUN4I) += \
>>   	sun4i-a10-mk802.dtb \
>>   	sun4i-a10-mk802ii.dtb \
>>   	sun4i-a10-olinuxino-lime.dtb \
>> -	sun4i-a10-pcduino.dtb
>> +	sun4i-a10-pcduino.dtb \
>> +	sun4i-a10-pov-protab2-ips9.dtb
>>   dtb-$(CONFIG_MACH_SUN5I) += \
>>   	sun5i-a10s-auxtek-t003.dtb \
>>   	sun5i-a10s-auxtek-t004.dtb \
>> diff --git a/arch/arm/boot/dts/sun4i-a10-pov-protab2-ips9.dts b/arch/arm/boot/dts/sun4i-a10-pov-protab2-ips9.dts
>> new file mode 100644
>> index 0000000..223515e
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/sun4i-a10-pov-protab2-ips9.dts
>> @@ -0,0 +1,209 @@
>> +/*
>> + * Copyright 2015 Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> + *
>> + * This file is dual-licensed: you can use it either under the terms
>> + * of the GPL or the X11 license, at your option. Note that this dual
>> + * licensing only applies to this file, and not this project as a
>> + * whole.
>> + *
>> + *  a) This file is free software; you can redistribute it and/or
>> + *     modify it under the terms of the GNU General Public License as
>> + *     published by the Free Software Foundation; either version 2 of the
>> + *     License, or (at your option) any later version.
>> + *
>> + *     This file is distributed in the hope that it will be useful,
>> + *     but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + *     GNU General Public License for more details.
>> + *
>> + * Or, alternatively,
>> + *
>> + *  b) Permission is hereby granted, free of charge, to any person
>> + *     obtaining a copy of this software and associated documentation
>> + *     files (the "Software"), to deal in the Software without
>> + *     restriction, including without limitation the rights to use,
>> + *     copy, modify, merge, publish, distribute, sublicense, and/or
>> + *     sell copies of the Software, and to permit persons to whom the
>> + *     Software is furnished to do so, subject to the following
>> + *     conditions:
>> + *
>> + *     The above copyright notice and this permission notice shall be
>> + *     included in all copies or substantial portions of the Software.
>> + *
>> + *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> + *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
>> + *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>> + *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
>> + *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
>> + *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>> + *     OTHER DEALINGS IN THE SOFTWARE.
>> + */
>> +
>> +/dts-v1/;
>> +#include "sun4i-a10.dtsi"
>> +#include "sunxi-common-regulators.dtsi"
>> +#include <dt-bindings/gpio/gpio.h>
>> +#include <dt-bindings/input/input.h>
>> +#include <dt-bindings/interrupt-controller/irq.h>
>> +#include <dt-bindings/pinctrl/sun4i-a10.h>
>> +
>> +/ {
>> +	model = "Point of View Protab2-IPS9";
>> +	compatible = "pov,protab2-ips9", "allwinner,sun4i-a10";
>> +
>> +	aliases {
>> +		serial0 = &uart0;
>> +	};
>> +
>> +	chosen {
>> +		stdout-path = "serial0:115200n8";
>> +	};
>> +};
>> +
>> +&cpu0 {
>> +	cpu-supply = <&reg_dcdc2>;
>> +};
>> +
>> +&ehci0 {
>> +	status = "okay";
>> +};
>> +
>> +&i2c0 {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&i2c0_pins_a>;
>> +	status = "okay";
>> +
>> +	axp209: pmic@34 {
>> +		reg = <0x34>;
>> +		interrupts = <0>;
>> +	};
>> +};
>> +
>> +#include "axp209.dtsi"
>> +
>> +&i2c1 {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&i2c1_pins_a>;
>> +	status = "okay";
>> +};
>> +
>> +&i2c2 {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&i2c2_pins_a>;
>> +	status = "okay";
>> +};
>> +
>> +&lradc {
>> +	vref-supply = <&reg_ldo2>;
>> +	status = "okay";
>> +
>> +	button@400 {
>> +		label = "Volume Up";
>> +		linux,code = <KEY_VOLUMEUP>;
>> +		channel = <0>;
>> +		voltage = <400000>;
>> +	};
>> +
>> +	button@800 {
>> +		label = "Volume Down";
>> +		linux,code = <KEY_VOLUMEDOWN>;
>> +		channel = <0>;
>> +		voltage = <800000>;
>> +	};
>> +};
>> +
>> +&mmc0 {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&mmc0_pins_a>, <&mmc0_cd_pin_reference_design>;
>> +	vmmc-supply = <&reg_vcc3v3>;
>> +	bus-width = <4>;
>> +	cd-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>; /* PH1 */
>> +	cd-inverted;
>> +	status = "okay";
>> +};
>> +
>> +&otg_sram {
>> +	status = "okay";
>> +};
>> +
>> +&pio {
>> +	usb0_id_detect_pin: usb0_id_detect_pin@0 {
>> +		allwinner,pins = "PH4";
>> +		allwinner,function = "gpio_in";
>> +		allwinner,drive = <SUN4I_PINCTRL_10_MA>;
>> +		allwinner,pull = <SUN4I_PINCTRL_PULL_UP>;
>> +	};
>> +
>> +	usb0_vbus_detect_pin: usb0_vbus_detect_pin@0 {
>> +		allwinner,pins = "PH5";
>> +		allwinner,function = "gpio_in";
>> +		allwinner,drive = <SUN4I_PINCTRL_10_MA>;
>> +		allwinner,pull = <SUN4I_PINCTRL_PULL_DOWN>;
>> +	};
>> +};
>> +
>> +&reg_dcdc2 {
>> +	regulator-always-on;
>> +	regulator-min-microvolt = <1000000>;
>> +	regulator-max-microvolt = <1400000>;
>> +	regulator-name = "vdd-cpu";
>> +};
>> +
>> +&reg_dcdc3 {
>> +	regulator-always-on;
>> +	regulator-min-microvolt = <1250000>;
>> +	regulator-max-microvolt = <1250000>;
>> +	regulator-name = "vdd-int-dll";
>> +};
>> +
>> +&reg_ldo1 {
>> +	regulator-name = "vdd-rtc";
>> +};
>> +
>> +&reg_ldo2 {
>> +	regulator-always-on;
>> +	regulator-min-microvolt = <3000000>;
>> +	regulator-max-microvolt = <3000000>;
>> +	regulator-name = "avcc";
>> +};
>> +
>> +&reg_ldo3 {
>> +	/*
>> +	 * We need to always power the camera sensor, otherwhise all access
>> +	 * to i2c1 is blocked.
>> +	 */
>> +	regulator-always-on;
>> +	regulator-min-microvolt = <2800000>;
>> +	regulator-max-microvolt = <2800000>;
>> +	regulator-name = "vdd-csi";
>> +};
>
> What is connected on i2c1 ? Just the camera sensor? or it has some
> other devices there?

The bma250 accelerometer sits there, and the kernel already has a driver
for it. That driver needs to have devicetree binding support added, and
then we should be able to use the accelerometer.

Regards,

Hans

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [linux-sunxi] Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
@ 2015-09-07  7:05       ` Hans de Goede
  0 siblings, 0 replies; 44+ messages in thread
From: Hans de Goede @ 2015-09-07  7:05 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 06-09-15 18:30, Maxime Ripard wrote:
> On Sat, Sep 05, 2015 at 10:21:59AM +0200, Hans de Goede wrote:
>> The Point of View protab2-ips9 is a tablet with a 9" ips 1024x768 lcd screen,
>> microsd slot, headphones, mini hdmi, mini usb b and power barrel connectors.
>>
>> It uses a rtl8188cus usb wifi chip and a RDA 5875Y bluetooth chip attached
>> to uart2. It has a bma250 accelerometer attached to i2c1 addr 0x18.
>>
>> It has a pixcir,pixcir_tangoc compatible touchscreen attached to i2c2 addr
>> 0x5c. This is not enabled in this dts, because this variant of the
>> pixcir_tangoc has separate wakeup and enable pins both of which need
>> to be driven low before the touchscreen will work. Before we can enable
>> this the pixcir driver and devicetree-bindings need to be extended to
>> support these pins.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   arch/arm/boot/dts/Makefile                       |   3 +-
>>   arch/arm/boot/dts/sun4i-a10-pov-protab2-ips9.dts | 209 +++++++++++++++++++++++
>>   2 files changed, 211 insertions(+), 1 deletion(-)
>>   create mode 100644 arch/arm/boot/dts/sun4i-a10-pov-protab2-ips9.dts
>>
>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>> index e981fd6..c08883c 100644
>> --- a/arch/arm/boot/dts/Makefile
>> +++ b/arch/arm/boot/dts/Makefile
>> @@ -561,7 +561,8 @@ dtb-$(CONFIG_MACH_SUN4I) += \
>>   	sun4i-a10-mk802.dtb \
>>   	sun4i-a10-mk802ii.dtb \
>>   	sun4i-a10-olinuxino-lime.dtb \
>> -	sun4i-a10-pcduino.dtb
>> +	sun4i-a10-pcduino.dtb \
>> +	sun4i-a10-pov-protab2-ips9.dtb
>>   dtb-$(CONFIG_MACH_SUN5I) += \
>>   	sun5i-a10s-auxtek-t003.dtb \
>>   	sun5i-a10s-auxtek-t004.dtb \
>> diff --git a/arch/arm/boot/dts/sun4i-a10-pov-protab2-ips9.dts b/arch/arm/boot/dts/sun4i-a10-pov-protab2-ips9.dts
>> new file mode 100644
>> index 0000000..223515e
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/sun4i-a10-pov-protab2-ips9.dts
>> @@ -0,0 +1,209 @@
>> +/*
>> + * Copyright 2015 Hans de Goede <hdegoede@redhat.com>
>> + *
>> + * This file is dual-licensed: you can use it either under the terms
>> + * of the GPL or the X11 license, at your option. Note that this dual
>> + * licensing only applies to this file, and not this project as a
>> + * whole.
>> + *
>> + *  a) This file is free software; you can redistribute it and/or
>> + *     modify it under the terms of the GNU General Public License as
>> + *     published by the Free Software Foundation; either version 2 of the
>> + *     License, or (at your option) any later version.
>> + *
>> + *     This file is distributed in the hope that it will be useful,
>> + *     but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + *     GNU General Public License for more details.
>> + *
>> + * Or, alternatively,
>> + *
>> + *  b) Permission is hereby granted, free of charge, to any person
>> + *     obtaining a copy of this software and associated documentation
>> + *     files (the "Software"), to deal in the Software without
>> + *     restriction, including without limitation the rights to use,
>> + *     copy, modify, merge, publish, distribute, sublicense, and/or
>> + *     sell copies of the Software, and to permit persons to whom the
>> + *     Software is furnished to do so, subject to the following
>> + *     conditions:
>> + *
>> + *     The above copyright notice and this permission notice shall be
>> + *     included in all copies or substantial portions of the Software.
>> + *
>> + *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> + *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
>> + *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>> + *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
>> + *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
>> + *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>> + *     OTHER DEALINGS IN THE SOFTWARE.
>> + */
>> +
>> +/dts-v1/;
>> +#include "sun4i-a10.dtsi"
>> +#include "sunxi-common-regulators.dtsi"
>> +#include <dt-bindings/gpio/gpio.h>
>> +#include <dt-bindings/input/input.h>
>> +#include <dt-bindings/interrupt-controller/irq.h>
>> +#include <dt-bindings/pinctrl/sun4i-a10.h>
>> +
>> +/ {
>> +	model = "Point of View Protab2-IPS9";
>> +	compatible = "pov,protab2-ips9", "allwinner,sun4i-a10";
>> +
>> +	aliases {
>> +		serial0 = &uart0;
>> +	};
>> +
>> +	chosen {
>> +		stdout-path = "serial0:115200n8";
>> +	};
>> +};
>> +
>> +&cpu0 {
>> +	cpu-supply = <&reg_dcdc2>;
>> +};
>> +
>> +&ehci0 {
>> +	status = "okay";
>> +};
>> +
>> +&i2c0 {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&i2c0_pins_a>;
>> +	status = "okay";
>> +
>> +	axp209: pmic at 34 {
>> +		reg = <0x34>;
>> +		interrupts = <0>;
>> +	};
>> +};
>> +
>> +#include "axp209.dtsi"
>> +
>> +&i2c1 {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&i2c1_pins_a>;
>> +	status = "okay";
>> +};
>> +
>> +&i2c2 {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&i2c2_pins_a>;
>> +	status = "okay";
>> +};
>> +
>> +&lradc {
>> +	vref-supply = <&reg_ldo2>;
>> +	status = "okay";
>> +
>> +	button at 400 {
>> +		label = "Volume Up";
>> +		linux,code = <KEY_VOLUMEUP>;
>> +		channel = <0>;
>> +		voltage = <400000>;
>> +	};
>> +
>> +	button at 800 {
>> +		label = "Volume Down";
>> +		linux,code = <KEY_VOLUMEDOWN>;
>> +		channel = <0>;
>> +		voltage = <800000>;
>> +	};
>> +};
>> +
>> +&mmc0 {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&mmc0_pins_a>, <&mmc0_cd_pin_reference_design>;
>> +	vmmc-supply = <&reg_vcc3v3>;
>> +	bus-width = <4>;
>> +	cd-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>; /* PH1 */
>> +	cd-inverted;
>> +	status = "okay";
>> +};
>> +
>> +&otg_sram {
>> +	status = "okay";
>> +};
>> +
>> +&pio {
>> +	usb0_id_detect_pin: usb0_id_detect_pin at 0 {
>> +		allwinner,pins = "PH4";
>> +		allwinner,function = "gpio_in";
>> +		allwinner,drive = <SUN4I_PINCTRL_10_MA>;
>> +		allwinner,pull = <SUN4I_PINCTRL_PULL_UP>;
>> +	};
>> +
>> +	usb0_vbus_detect_pin: usb0_vbus_detect_pin at 0 {
>> +		allwinner,pins = "PH5";
>> +		allwinner,function = "gpio_in";
>> +		allwinner,drive = <SUN4I_PINCTRL_10_MA>;
>> +		allwinner,pull = <SUN4I_PINCTRL_PULL_DOWN>;
>> +	};
>> +};
>> +
>> +&reg_dcdc2 {
>> +	regulator-always-on;
>> +	regulator-min-microvolt = <1000000>;
>> +	regulator-max-microvolt = <1400000>;
>> +	regulator-name = "vdd-cpu";
>> +};
>> +
>> +&reg_dcdc3 {
>> +	regulator-always-on;
>> +	regulator-min-microvolt = <1250000>;
>> +	regulator-max-microvolt = <1250000>;
>> +	regulator-name = "vdd-int-dll";
>> +};
>> +
>> +&reg_ldo1 {
>> +	regulator-name = "vdd-rtc";
>> +};
>> +
>> +&reg_ldo2 {
>> +	regulator-always-on;
>> +	regulator-min-microvolt = <3000000>;
>> +	regulator-max-microvolt = <3000000>;
>> +	regulator-name = "avcc";
>> +};
>> +
>> +&reg_ldo3 {
>> +	/*
>> +	 * We need to always power the camera sensor, otherwhise all access
>> +	 * to i2c1 is blocked.
>> +	 */
>> +	regulator-always-on;
>> +	regulator-min-microvolt = <2800000>;
>> +	regulator-max-microvolt = <2800000>;
>> +	regulator-name = "vdd-csi";
>> +};
>
> What is connected on i2c1 ? Just the camera sensor? or it has some
> other devices there?

The bma250 accelerometer sits there, and the kernel already has a driver
for it. That driver needs to have devicetree binding support added, and
then we should be able to use the accelerometer.

Regards,

Hans

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
  2015-09-07  7:05       ` [linux-sunxi] " Hans de Goede
@ 2015-09-07  7:49           ` Priit Laes
  -1 siblings, 0 replies; 44+ messages in thread
From: Priit Laes @ 2015-09-07  7:49 UTC (permalink / raw)
  To: hdegoede-H+wXaHxf7aLQT0dZR+AlfA, Maxime Ripard
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw

On Mon, 2015-09-07 at 09:05 +0200, Hans de Goede wrote:
> Hi,
> 
> On 06-09-15 18:30, Maxime Ripard wrote:
> > On Sat, Sep 05, 2015 at 10:21:59AM +0200, Hans de Goede wrote:
> > > The Point of View protab2-ips9 is a tablet with a 9" ips 1024x768
> > > lcd screen,
> > > microsd slot, headphones, mini hdmi, mini usb b and power barrel
> > > connectors.
> > > 
> > > It uses a rtl8188cus usb wifi chip and a RDA 5875Y bluetooth chip
> > > attached
> > > to uart2. It has a bma250 accelerometer attached to i2c1 addr
> > > 0x18.
> > > 

[...]
> > 
> > What is connected on i2c1 ? Just the camera sensor? or it has some
> > other devices there?
> 
> The bma250 accelerometer sits there, and the kernel already has a
> driver
> for it. That driver needs to have devicetree binding support added,
> and
> then we should be able to use the accelerometer.

bma250 already has devicetree support. It is used in Gemei G9 tablet
(sun4i-gemei-g9.dts).

Päikest,
Priit :)

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [linux-sunxi] Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
@ 2015-09-07  7:49           ` Priit Laes
  0 siblings, 0 replies; 44+ messages in thread
From: Priit Laes @ 2015-09-07  7:49 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, 2015-09-07 at 09:05 +0200, Hans de Goede wrote:
> Hi,
> 
> On 06-09-15 18:30, Maxime Ripard wrote:
> > On Sat, Sep 05, 2015 at 10:21:59AM +0200, Hans de Goede wrote:
> > > The Point of View protab2-ips9 is a tablet with a 9" ips 1024x768
> > > lcd screen,
> > > microsd slot, headphones, mini hdmi, mini usb b and power barrel
> > > connectors.
> > > 
> > > It uses a rtl8188cus usb wifi chip and a RDA 5875Y bluetooth chip
> > > attached
> > > to uart2. It has a bma250 accelerometer attached to i2c1 addr
> > > 0x18.
> > > 

[...]
> > 
> > What is connected on i2c1 ? Just the camera sensor? or it has some
> > other devices there?
> 
> The bma250 accelerometer sits there, and the kernel already has a
> driver
> for it. That driver needs to have devicetree binding support added,
> and
> then we should be able to use the accelerometer.

bma250 already has devicetree support. It is used in Gemei G9 tablet
(sun4i-gemei-g9.dts).

P?ikest,
Priit :)

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [linux-sunxi] Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
  2015-09-07  7:49           ` [linux-sunxi] " Priit Laes
@ 2015-09-07  8:49               ` Hans de Goede
  -1 siblings, 0 replies; 44+ messages in thread
From: Hans de Goede @ 2015-09-07  8:49 UTC (permalink / raw)
  To: plaes-q/aMd4JkU83YtjvyW6yDsg, Maxime Ripard
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw

Hi,

On 07-09-15 09:49, Priit Laes wrote:
> On Mon, 2015-09-07 at 09:05 +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 06-09-15 18:30, Maxime Ripard wrote:
>>> On Sat, Sep 05, 2015 at 10:21:59AM +0200, Hans de Goede wrote:
>>>> The Point of View protab2-ips9 is a tablet with a 9" ips 1024x768
>>>> lcd screen,
>>>> microsd slot, headphones, mini hdmi, mini usb b and power barrel
>>>> connectors.
>>>>
>>>> It uses a rtl8188cus usb wifi chip and a RDA 5875Y bluetooth chip
>>>> attached
>>>> to uart2. It has a bma250 accelerometer attached to i2c1 addr
>>>> 0x18.
>>>>
>
> [...]
>>>
>>> What is connected on i2c1 ? Just the camera sensor? or it has some
>>> other devices there?
>>
>> The bma250 accelerometer sits there, and the kernel already has a
>> driver
>> for it. That driver needs to have devicetree binding support added,
>> and
>> then we should be able to use the accelerometer.
>
> bma250 already has devicetree support. It is used in Gemei G9 tablet
> (sun4i-gemei-g9.dts).

Yes I've seen that, but does it actually work? I've not tried but I
do not see any compatible string in the actual bma250 code in the kernel,
so I believe that this part of the sun4i-gemei-g9.dts file does not work ?

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [linux-sunxi] Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
@ 2015-09-07  8:49               ` Hans de Goede
  0 siblings, 0 replies; 44+ messages in thread
From: Hans de Goede @ 2015-09-07  8:49 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 07-09-15 09:49, Priit Laes wrote:
> On Mon, 2015-09-07 at 09:05 +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 06-09-15 18:30, Maxime Ripard wrote:
>>> On Sat, Sep 05, 2015 at 10:21:59AM +0200, Hans de Goede wrote:
>>>> The Point of View protab2-ips9 is a tablet with a 9" ips 1024x768
>>>> lcd screen,
>>>> microsd slot, headphones, mini hdmi, mini usb b and power barrel
>>>> connectors.
>>>>
>>>> It uses a rtl8188cus usb wifi chip and a RDA 5875Y bluetooth chip
>>>> attached
>>>> to uart2. It has a bma250 accelerometer attached to i2c1 addr
>>>> 0x18.
>>>>
>
> [...]
>>>
>>> What is connected on i2c1 ? Just the camera sensor? or it has some
>>> other devices there?
>>
>> The bma250 accelerometer sits there, and the kernel already has a
>> driver
>> for it. That driver needs to have devicetree binding support added,
>> and
>> then we should be able to use the accelerometer.
>
> bma250 already has devicetree support. It is used in Gemei G9 tablet
> (sun4i-gemei-g9.dts).

Yes I've seen that, but does it actually work? I've not tried but I
do not see any compatible string in the actual bma250 code in the kernel,
so I believe that this part of the sun4i-gemei-g9.dts file does not work ?

Regards,

Hans

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [linux-sunxi] Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
  2015-09-07  8:49               ` Hans de Goede
@ 2015-09-07  9:14                   ` Priit Laes
  -1 siblings, 0 replies; 44+ messages in thread
From: Priit Laes @ 2015-09-07  9:14 UTC (permalink / raw)
  To: hdegoede-H+wXaHxf7aLQT0dZR+AlfA, Maxime Ripard
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw

On Mon, 2015-09-07 at 10:49 +0200, Hans de Goede wrote:
> Hi,
> 
> On 07-09-15 09:49, Priit Laes wrote:
> > On Mon, 2015-09-07 at 09:05 +0200, Hans de Goede wrote:
> > > Hi,
> > > 
> > > On 06-09-15 18:30, Maxime Ripard wrote:
> > > > On Sat, Sep 05, 2015 at 10:21:59AM +0200, Hans de Goede wrote:
> > > > > The Point of View protab2-ips9 is a tablet with a 9" ips
> > > > > 1024x768
> > > > > lcd screen,
> > > > > microsd slot, headphones, mini hdmi, mini usb b and power
> > > > > barrel
> > > > > connectors.
> > > > > 
> > > > > It uses a rtl8188cus usb wifi chip and a RDA 5875Y bluetooth
> > > > > chip
> > > > > attached
> > > > > to uart2. It has a bma250 accelerometer attached to i2c1 addr
> > > > > 0x18.
> > > > > 
> > 
> > [...]
> > > > 
> > > > What is connected on i2c1 ? Just the camera sensor? or it has
> > > > some
> > > > other devices there?
> > > 
> > > The bma250 accelerometer sits there, and the kernel already has a
> > > driver
> > > for it. That driver needs to have devicetree binding support
> > > added,
> > > and
> > > then we should be able to use the accelerometer.
> > 
> > bma250 already has devicetree support. It is used in Gemei G9
> > tablet
> > (sun4i-gemei-g9.dts).
> 
> Yes I've seen that, but does it actually work? I've not tried but I
> do not see any compatible string in the actual bma250 code in the
> kernel,
> so I believe that this part of the sun4i-gemei-g9.dts file does not
> work ?

It worked (even without IRQs) when I submitted the patch. Driver itself
is under iio/accel/bma180.c

Päikest,
Priit

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [linux-sunxi] Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
@ 2015-09-07  9:14                   ` Priit Laes
  0 siblings, 0 replies; 44+ messages in thread
From: Priit Laes @ 2015-09-07  9:14 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, 2015-09-07 at 10:49 +0200, Hans de Goede wrote:
> Hi,
> 
> On 07-09-15 09:49, Priit Laes wrote:
> > On Mon, 2015-09-07 at 09:05 +0200, Hans de Goede wrote:
> > > Hi,
> > > 
> > > On 06-09-15 18:30, Maxime Ripard wrote:
> > > > On Sat, Sep 05, 2015 at 10:21:59AM +0200, Hans de Goede wrote:
> > > > > The Point of View protab2-ips9 is a tablet with a 9" ips
> > > > > 1024x768
> > > > > lcd screen,
> > > > > microsd slot, headphones, mini hdmi, mini usb b and power
> > > > > barrel
> > > > > connectors.
> > > > > 
> > > > > It uses a rtl8188cus usb wifi chip and a RDA 5875Y bluetooth
> > > > > chip
> > > > > attached
> > > > > to uart2. It has a bma250 accelerometer attached to i2c1 addr
> > > > > 0x18.
> > > > > 
> > 
> > [...]
> > > > 
> > > > What is connected on i2c1 ? Just the camera sensor? or it has
> > > > some
> > > > other devices there?
> > > 
> > > The bma250 accelerometer sits there, and the kernel already has a
> > > driver
> > > for it. That driver needs to have devicetree binding support
> > > added,
> > > and
> > > then we should be able to use the accelerometer.
> > 
> > bma250 already has devicetree support. It is used in Gemei G9
> > tablet
> > (sun4i-gemei-g9.dts).
> 
> Yes I've seen that, but does it actually work? I've not tried but I
> do not see any compatible string in the actual bma250 code in the
> kernel,
> so I believe that this part of the sun4i-gemei-g9.dts file does not
> work ?

It worked (even without IRQs) when I submitted the patch. Driver itself
is under iio/accel/bma180.c

P?ikest,
Priit

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [linux-sunxi] Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
  2015-09-07  9:14                   ` Priit Laes
@ 2015-09-07  9:30                       ` Hans de Goede
  -1 siblings, 0 replies; 44+ messages in thread
From: Hans de Goede @ 2015-09-07  9:30 UTC (permalink / raw)
  To: Priit Laes, Maxime Ripard
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw

Hi,

On 07-09-15 11:14, Priit Laes wrote:
> On Mon, 2015-09-07 at 10:49 +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 07-09-15 09:49, Priit Laes wrote:
>>> On Mon, 2015-09-07 at 09:05 +0200, Hans de Goede wrote:
>>>> Hi,
>>>>
>>>> On 06-09-15 18:30, Maxime Ripard wrote:
>>>>> On Sat, Sep 05, 2015 at 10:21:59AM +0200, Hans de Goede wrote:
>>>>>> The Point of View protab2-ips9 is a tablet with a 9" ips
>>>>>> 1024x768
>>>>>> lcd screen,
>>>>>> microsd slot, headphones, mini hdmi, mini usb b and power
>>>>>> barrel
>>>>>> connectors.
>>>>>>
>>>>>> It uses a rtl8188cus usb wifi chip and a RDA 5875Y bluetooth
>>>>>> chip
>>>>>> attached
>>>>>> to uart2. It has a bma250 accelerometer attached to i2c1 addr
>>>>>> 0x18.
>>>>>>
>>>
>>> [...]
>>>>>
>>>>> What is connected on i2c1 ? Just the camera sensor? or it has
>>>>> some
>>>>> other devices there?
>>>>
>>>> The bma250 accelerometer sits there, and the kernel already has a
>>>> driver
>>>> for it. That driver needs to have devicetree binding support
>>>> added,
>>>> and
>>>> then we should be able to use the accelerometer.
>>>
>>> bma250 already has devicetree support. It is used in Gemei G9
>>> tablet
>>> (sun4i-gemei-g9.dts).
>>
>> Yes I've seen that, but does it actually work? I've not tried but I
>> do not see any compatible string in the actual bma250 code in the
>> kernel,
>> so I believe that this part of the sun4i-gemei-g9.dts file does not
>> work ?
>
> It worked (even without IRQs) when I submitted the patch. Driver itself
> is under iio/accel/bma180.c

That is really weird, because:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/iio/accel/bma180.c

Does not have an of_match_table ... ??

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [linux-sunxi] Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
@ 2015-09-07  9:30                       ` Hans de Goede
  0 siblings, 0 replies; 44+ messages in thread
From: Hans de Goede @ 2015-09-07  9:30 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 07-09-15 11:14, Priit Laes wrote:
> On Mon, 2015-09-07 at 10:49 +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 07-09-15 09:49, Priit Laes wrote:
>>> On Mon, 2015-09-07 at 09:05 +0200, Hans de Goede wrote:
>>>> Hi,
>>>>
>>>> On 06-09-15 18:30, Maxime Ripard wrote:
>>>>> On Sat, Sep 05, 2015 at 10:21:59AM +0200, Hans de Goede wrote:
>>>>>> The Point of View protab2-ips9 is a tablet with a 9" ips
>>>>>> 1024x768
>>>>>> lcd screen,
>>>>>> microsd slot, headphones, mini hdmi, mini usb b and power
>>>>>> barrel
>>>>>> connectors.
>>>>>>
>>>>>> It uses a rtl8188cus usb wifi chip and a RDA 5875Y bluetooth
>>>>>> chip
>>>>>> attached
>>>>>> to uart2. It has a bma250 accelerometer attached to i2c1 addr
>>>>>> 0x18.
>>>>>>
>>>
>>> [...]
>>>>>
>>>>> What is connected on i2c1 ? Just the camera sensor? or it has
>>>>> some
>>>>> other devices there?
>>>>
>>>> The bma250 accelerometer sits there, and the kernel already has a
>>>> driver
>>>> for it. That driver needs to have devicetree binding support
>>>> added,
>>>> and
>>>> then we should be able to use the accelerometer.
>>>
>>> bma250 already has devicetree support. It is used in Gemei G9
>>> tablet
>>> (sun4i-gemei-g9.dts).
>>
>> Yes I've seen that, but does it actually work? I've not tried but I
>> do not see any compatible string in the actual bma250 code in the
>> kernel,
>> so I believe that this part of the sun4i-gemei-g9.dts file does not
>> work ?
>
> It worked (even without IRQs) when I submitted the patch. Driver itself
> is under iio/accel/bma180.c

That is really weird, because:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/iio/accel/bma180.c

Does not have an of_match_table ... ??

Regards,

Hans

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
  2015-09-07  9:30                       ` Hans de Goede
@ 2015-09-07 20:52                           ` Maxime Ripard
  -1 siblings, 0 replies; 44+ messages in thread
From: Maxime Ripard @ 2015-09-07 20:52 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Priit Laes, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw

[-- Attachment #1: Type: text/plain, Size: 1271 bytes --]

Hi,

On Mon, Sep 07, 2015 at 11:30:03AM +0200, Hans de Goede wrote:
> >>>bma250 already has devicetree support. It is used in Gemei G9
> >>>tablet (sun4i-gemei-g9.dts).
> >>
> >>Yes I've seen that, but does it actually work? I've not tried but
> >>I do not see any compatible string in the actual bma250 code in
> >>the kernel, so I believe that this part of the sun4i-gemei-g9.dts
> >>file does not work ?
> >
> >It worked (even without IRQs) when I submitted the patch. Driver
> >itself is under iio/accel/bma180.c
> 
> That is really weird, because:
> 
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/iio/accel/bma180.c
> 
> Does not have an of_match_table ... ??

Not really, when using DT, i2c will set the i2c_client name to the
device part of the compatible [1] [2], and then if the of_device_id
lookup fails, will fallback to matching the i2c_client name to the
i2c_device_id [3]. Which in our case matches.

Maxime

[1] http://lxr.free-electrons.com/source/drivers/i2c/i2c-core.c#L1281
[2] http://lxr.free-electrons.com/source/drivers/i2c/i2c-core.c#L969
[3] http://lxr.free-electrons.com/source/drivers/i2c/i2c-core.c#L461

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [linux-sunxi] Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
@ 2015-09-07 20:52                           ` Maxime Ripard
  0 siblings, 0 replies; 44+ messages in thread
From: Maxime Ripard @ 2015-09-07 20:52 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Mon, Sep 07, 2015 at 11:30:03AM +0200, Hans de Goede wrote:
> >>>bma250 already has devicetree support. It is used in Gemei G9
> >>>tablet (sun4i-gemei-g9.dts).
> >>
> >>Yes I've seen that, but does it actually work? I've not tried but
> >>I do not see any compatible string in the actual bma250 code in
> >>the kernel, so I believe that this part of the sun4i-gemei-g9.dts
> >>file does not work ?
> >
> >It worked (even without IRQs) when I submitted the patch. Driver
> >itself is under iio/accel/bma180.c
> 
> That is really weird, because:
> 
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/iio/accel/bma180.c
> 
> Does not have an of_match_table ... ??

Not really, when using DT, i2c will set the i2c_client name to the
device part of the compatible [1] [2], and then if the of_device_id
lookup fails, will fallback to matching the i2c_client name to the
i2c_device_id [3]. Which in our case matches.

Maxime

[1] http://lxr.free-electrons.com/source/drivers/i2c/i2c-core.c#L1281
[2] http://lxr.free-electrons.com/source/drivers/i2c/i2c-core.c#L969
[3] http://lxr.free-electrons.com/source/drivers/i2c/i2c-core.c#L461

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150907/c433c2b9/attachment.sig>

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
  2015-09-07  7:05       ` [linux-sunxi] " Hans de Goede
@ 2015-09-07 20:56           ` Maxime Ripard
  -1 siblings, 0 replies; 44+ messages in thread
From: Maxime Ripard @ 2015-09-07 20:56 UTC (permalink / raw)
  To: Hans de Goede, Wolfram Sang
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw

[-- Attachment #1: Type: text/plain, Size: 1071 bytes --]

On Mon, Sep 07, 2015 at 09:05:29AM +0200, Hans de Goede wrote:
> >>+&reg_ldo3 {
> >>+	/*
> >>+	 * We need to always power the camera sensor, otherwhise all access
> >>+	 * to i2c1 is blocked.
> >>+	 */
> >>+	regulator-always-on;
> >>+	regulator-min-microvolt = <2800000>;
> >>+	regulator-max-microvolt = <2800000>;
> >>+	regulator-name = "vdd-csi";
> >>+};
> >
> >What is connected on i2c1 ? Just the camera sensor? or it has some
> >other devices there?
> 
> The bma250 accelerometer sits there, and the kernel already has a driver
> for it. That driver needs to have devicetree binding support added, and
> then we should be able to use the accelerometer.

Ok, so if this regulator is disable, you can't access the other
devices as well, right? Do you know why? Is it the regulator providing
the pull-up voltage?

I'd still like Wolfram's insight on this one. Wolfram, do you have a
suggestion and / or a comment on how to deal with such a case?

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [linux-sunxi] Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
@ 2015-09-07 20:56           ` Maxime Ripard
  0 siblings, 0 replies; 44+ messages in thread
From: Maxime Ripard @ 2015-09-07 20:56 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Sep 07, 2015 at 09:05:29AM +0200, Hans de Goede wrote:
> >>+&reg_ldo3 {
> >>+	/*
> >>+	 * We need to always power the camera sensor, otherwhise all access
> >>+	 * to i2c1 is blocked.
> >>+	 */
> >>+	regulator-always-on;
> >>+	regulator-min-microvolt = <2800000>;
> >>+	regulator-max-microvolt = <2800000>;
> >>+	regulator-name = "vdd-csi";
> >>+};
> >
> >What is connected on i2c1 ? Just the camera sensor? or it has some
> >other devices there?
> 
> The bma250 accelerometer sits there, and the kernel already has a driver
> for it. That driver needs to have devicetree binding support added, and
> then we should be able to use the accelerometer.

Ok, so if this regulator is disable, you can't access the other
devices as well, right? Do you know why? Is it the regulator providing
the pull-up voltage?

I'd still like Wolfram's insight on this one. Wolfram, do you have a
suggestion and / or a comment on how to deal with such a case?

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150907/47f3fe4a/attachment.sig>

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [linux-sunxi] Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
  2015-09-07 20:56           ` [linux-sunxi] " Maxime Ripard
@ 2015-09-08  7:45             ` Hans de Goede
  -1 siblings, 0 replies; 44+ messages in thread
From: Hans de Goede @ 2015-09-08  7:45 UTC (permalink / raw)
  To: Maxime Ripard, Wolfram Sang
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw

Hi,

On 07-09-15 22:56, Maxime Ripard wrote:
> On Mon, Sep 07, 2015 at 09:05:29AM +0200, Hans de Goede wrote:
>>>> +&reg_ldo3 {
>>>> +	/*
>>>> +	 * We need to always power the camera sensor, otherwhise all access
>>>> +	 * to i2c1 is blocked.
>>>> +	 */
>>>> +	regulator-always-on;
>>>> +	regulator-min-microvolt = <2800000>;
>>>> +	regulator-max-microvolt = <2800000>;
>>>> +	regulator-name = "vdd-csi";
>>>> +};
>>>
>>> What is connected on i2c1 ? Just the camera sensor? or it has some
>>> other devices there?
>>
>> The bma250 accelerometer sits there, and the kernel already has a driver
>> for it. That driver needs to have devicetree binding support added, and
>> then we should be able to use the accelerometer.
>
> Ok, so if this regulator is disable, you can't access the other
> devices as well, right?

Right, the controller reports the bus as being stuck.

> Do you know why? Is it the regulator providing
> the pull-up voltage?

I've tried enabling the pull ups on the SoC i2c pins, so I do not think
that it is that, it seems that somehow when not powered the camera sensor is
actively keeping the lines low. Either it has multiple power planes, or
it is using normally-on fet-s between ground and its i2c lines.

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [linux-sunxi] Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
@ 2015-09-08  7:45             ` Hans de Goede
  0 siblings, 0 replies; 44+ messages in thread
From: Hans de Goede @ 2015-09-08  7:45 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 07-09-15 22:56, Maxime Ripard wrote:
> On Mon, Sep 07, 2015 at 09:05:29AM +0200, Hans de Goede wrote:
>>>> +&reg_ldo3 {
>>>> +	/*
>>>> +	 * We need to always power the camera sensor, otherwhise all access
>>>> +	 * to i2c1 is blocked.
>>>> +	 */
>>>> +	regulator-always-on;
>>>> +	regulator-min-microvolt = <2800000>;
>>>> +	regulator-max-microvolt = <2800000>;
>>>> +	regulator-name = "vdd-csi";
>>>> +};
>>>
>>> What is connected on i2c1 ? Just the camera sensor? or it has some
>>> other devices there?
>>
>> The bma250 accelerometer sits there, and the kernel already has a driver
>> for it. That driver needs to have devicetree binding support added, and
>> then we should be able to use the accelerometer.
>
> Ok, so if this regulator is disable, you can't access the other
> devices as well, right?

Right, the controller reports the bus as being stuck.

> Do you know why? Is it the regulator providing
> the pull-up voltage?

I've tried enabling the pull ups on the SoC i2c pins, so I do not think
that it is that, it seems that somehow when not powered the camera sensor is
actively keeping the lines low. Either it has multiple power planes, or
it is using normally-on fet-s between ground and its i2c lines.

Regards,

Hans

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
  2015-09-07 20:52                           ` [linux-sunxi] " Maxime Ripard
@ 2015-09-08  7:48                             ` Hans de Goede
  -1 siblings, 0 replies; 44+ messages in thread
From: Hans de Goede @ 2015-09-08  7:48 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Priit Laes, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw

Hi,

On 07-09-15 22:52, Maxime Ripard wrote:
> Hi,
>
> On Mon, Sep 07, 2015 at 11:30:03AM +0200, Hans de Goede wrote:
>>>>> bma250 already has devicetree support. It is used in Gemei G9
>>>>> tablet (sun4i-gemei-g9.dts).
>>>>
>>>> Yes I've seen that, but does it actually work? I've not tried but
>>>> I do not see any compatible string in the actual bma250 code in
>>>> the kernel, so I believe that this part of the sun4i-gemei-g9.dts
>>>> file does not work ?
>>>
>>> It worked (even without IRQs) when I submitted the patch. Driver
>>> itself is under iio/accel/bma180.c
>>
>> That is really weird, because:
>>
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/iio/accel/bma180.c
>>
>> Does not have an of_match_table ... ??
>
> Not really, when using DT, i2c will set the i2c_client name to the
> device part of the compatible [1] [2], and then if the of_device_id
> lookup fails, will fallback to matching the i2c_client name to the
> i2c_device_id [3]. Which in our case matches.
>
> Maxime
>
> [1] http://lxr.free-electrons.com/source/drivers/i2c/i2c-core.c#L1281
> [2] http://lxr.free-electrons.com/source/drivers/i2c/i2c-core.c#L969
> [3] http://lxr.free-electrons.com/source/drivers/i2c/i2c-core.c#L461

Hmm, not sure whether that is a useful feature or an ugly hack :)

It is sorta unexpected either way. But I can make good use of this to
enable the accelerometer and maybe also some touchscreens on a bunch of
boards I have.

Still should we rely on this? This means relying on Linux kernel behavior,
rather then something documented in Documentation/devicetree/bindings/...

Regards,

Hans

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [linux-sunxi] Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
@ 2015-09-08  7:48                             ` Hans de Goede
  0 siblings, 0 replies; 44+ messages in thread
From: Hans de Goede @ 2015-09-08  7:48 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 07-09-15 22:52, Maxime Ripard wrote:
> Hi,
>
> On Mon, Sep 07, 2015 at 11:30:03AM +0200, Hans de Goede wrote:
>>>>> bma250 already has devicetree support. It is used in Gemei G9
>>>>> tablet (sun4i-gemei-g9.dts).
>>>>
>>>> Yes I've seen that, but does it actually work? I've not tried but
>>>> I do not see any compatible string in the actual bma250 code in
>>>> the kernel, so I believe that this part of the sun4i-gemei-g9.dts
>>>> file does not work ?
>>>
>>> It worked (even without IRQs) when I submitted the patch. Driver
>>> itself is under iio/accel/bma180.c
>>
>> That is really weird, because:
>>
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/iio/accel/bma180.c
>>
>> Does not have an of_match_table ... ??
>
> Not really, when using DT, i2c will set the i2c_client name to the
> device part of the compatible [1] [2], and then if the of_device_id
> lookup fails, will fallback to matching the i2c_client name to the
> i2c_device_id [3]. Which in our case matches.
>
> Maxime
>
> [1] http://lxr.free-electrons.com/source/drivers/i2c/i2c-core.c#L1281
> [2] http://lxr.free-electrons.com/source/drivers/i2c/i2c-core.c#L969
> [3] http://lxr.free-electrons.com/source/drivers/i2c/i2c-core.c#L461

Hmm, not sure whether that is a useful feature or an ugly hack :)

It is sorta unexpected either way. But I can make good use of this to
enable the accelerometer and maybe also some touchscreens on a bunch of
boards I have.

Still should we rely on this? This means relying on Linux kernel behavior,
rather then something documented in Documentation/devicetree/bindings/...

Regards,

Hans

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
  2015-09-08  7:45             ` Hans de Goede
@ 2015-09-08  8:40                 ` Chen-Yu Tsai
  -1 siblings, 0 replies; 44+ messages in thread
From: Chen-Yu Tsai @ 2015-09-08  8:40 UTC (permalink / raw)
  To: Hans De Goede
  Cc: Maxime Ripard, Wolfram Sang, linux-arm-kernel, devicetree, linux-sunxi

On Tue, Sep 8, 2015 at 3:45 PM, Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> Hi,
>
> On 07-09-15 22:56, Maxime Ripard wrote:
>>
>> On Mon, Sep 07, 2015 at 09:05:29AM +0200, Hans de Goede wrote:
>>>>>
>>>>> +&reg_ldo3 {
>>>>> +       /*
>>>>> +        * We need to always power the camera sensor, otherwhise all
>>>>> access
>>>>> +        * to i2c1 is blocked.
>>>>> +        */
>>>>> +       regulator-always-on;
>>>>> +       regulator-min-microvolt = <2800000>;
>>>>> +       regulator-max-microvolt = <2800000>;
>>>>> +       regulator-name = "vdd-csi";
>>>>> +};
>>>>
>>>>
>>>> What is connected on i2c1 ? Just the camera sensor? or it has some
>>>> other devices there?
>>>
>>>
>>> The bma250 accelerometer sits there, and the kernel already has a driver
>>> for it. That driver needs to have devicetree binding support added, and
>>> then we should be able to use the accelerometer.
>>
>>
>> Ok, so if this regulator is disable, you can't access the other
>> devices as well, right?
>
>
> Right, the controller reports the bus as being stuck.
>
>> Do you know why? Is it the regulator providing
>> the pull-up voltage?
>
>
> I've tried enabling the pull ups on the SoC i2c pins, so I do not think
> that it is that, it seems that somehow when not powered the camera sensor is
> actively keeping the lines low. Either it has multiple power planes, or
> it is using normally-on fet-s between ground and its i2c lines.

FYI the reference designs use one regulator to power the pull-ups, VCC-PX
(X for X pin group), and VDDIO (IO power)on the camera sensors. AVDD, DVDD
(actual power) for the sensors are another (or more) regulators.


ChenYu

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [linux-sunxi] Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
@ 2015-09-08  8:40                 ` Chen-Yu Tsai
  0 siblings, 0 replies; 44+ messages in thread
From: Chen-Yu Tsai @ 2015-09-08  8:40 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Sep 8, 2015 at 3:45 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> On 07-09-15 22:56, Maxime Ripard wrote:
>>
>> On Mon, Sep 07, 2015 at 09:05:29AM +0200, Hans de Goede wrote:
>>>>>
>>>>> +&reg_ldo3 {
>>>>> +       /*
>>>>> +        * We need to always power the camera sensor, otherwhise all
>>>>> access
>>>>> +        * to i2c1 is blocked.
>>>>> +        */
>>>>> +       regulator-always-on;
>>>>> +       regulator-min-microvolt = <2800000>;
>>>>> +       regulator-max-microvolt = <2800000>;
>>>>> +       regulator-name = "vdd-csi";
>>>>> +};
>>>>
>>>>
>>>> What is connected on i2c1 ? Just the camera sensor? or it has some
>>>> other devices there?
>>>
>>>
>>> The bma250 accelerometer sits there, and the kernel already has a driver
>>> for it. That driver needs to have devicetree binding support added, and
>>> then we should be able to use the accelerometer.
>>
>>
>> Ok, so if this regulator is disable, you can't access the other
>> devices as well, right?
>
>
> Right, the controller reports the bus as being stuck.
>
>> Do you know why? Is it the regulator providing
>> the pull-up voltage?
>
>
> I've tried enabling the pull ups on the SoC i2c pins, so I do not think
> that it is that, it seems that somehow when not powered the camera sensor is
> actively keeping the lines low. Either it has multiple power planes, or
> it is using normally-on fet-s between ground and its i2c lines.

FYI the reference designs use one regulator to power the pull-ups, VCC-PX
(X for X pin group), and VDDIO (IO power)on the camera sensors. AVDD, DVDD
(actual power) for the sensors are another (or more) regulators.


ChenYu

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [linux-sunxi] Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
  2015-09-08  8:40                 ` [linux-sunxi] " Chen-Yu Tsai
@ 2015-09-08 12:49                     ` Hans de Goede
  -1 siblings, 0 replies; 44+ messages in thread
From: Hans de Goede @ 2015-09-08 12:49 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Maxime Ripard, Wolfram Sang, linux-arm-kernel, devicetree, linux-sunxi

Hi,

On 09/08/2015 10:40 AM, Chen-Yu Tsai wrote:
> On Tue, Sep 8, 2015 at 3:45 PM, Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> Hi,
>>
>> On 07-09-15 22:56, Maxime Ripard wrote:
>>>
>>> On Mon, Sep 07, 2015 at 09:05:29AM +0200, Hans de Goede wrote:
>>>>>>
>>>>>> +&reg_ldo3 {
>>>>>> +       /*
>>>>>> +        * We need to always power the camera sensor, otherwhise all
>>>>>> access
>>>>>> +        * to i2c1 is blocked.
>>>>>> +        */
>>>>>> +       regulator-always-on;
>>>>>> +       regulator-min-microvolt = <2800000>;
>>>>>> +       regulator-max-microvolt = <2800000>;
>>>>>> +       regulator-name = "vdd-csi";
>>>>>> +};
>>>>>
>>>>>
>>>>> What is connected on i2c1 ? Just the camera sensor? or it has some
>>>>> other devices there?
>>>>
>>>>
>>>> The bma250 accelerometer sits there, and the kernel already has a driver
>>>> for it. That driver needs to have devicetree binding support added, and
>>>> then we should be able to use the accelerometer.
>>>
>>>
>>> Ok, so if this regulator is disable, you can't access the other
>>> devices as well, right?
>>
>>
>> Right, the controller reports the bus as being stuck.
>>
>>> Do you know why? Is it the regulator providing
>>> the pull-up voltage?
>>
>>
>> I've tried enabling the pull ups on the SoC i2c pins, so I do not think
>> that it is that, it seems that somehow when not powered the camera sensor is
>> actively keeping the lines low. Either it has multiple power planes, or
>> it is using normally-on fet-s between ground and its i2c lines.
>
> FYI the reference designs use one regulator to power the pull-ups, VCC-PX
> (X for X pin group), and VDDIO (IO power)on the camera sensors. AVDD, DVDD
> (actual power) for the sensors are another (or more) regulators.

But that is a A23 or some such generation thing, right? This is an A10 based
tablet!

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [linux-sunxi] Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
@ 2015-09-08 12:49                     ` Hans de Goede
  0 siblings, 0 replies; 44+ messages in thread
From: Hans de Goede @ 2015-09-08 12:49 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 09/08/2015 10:40 AM, Chen-Yu Tsai wrote:
> On Tue, Sep 8, 2015 at 3:45 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>> On 07-09-15 22:56, Maxime Ripard wrote:
>>>
>>> On Mon, Sep 07, 2015 at 09:05:29AM +0200, Hans de Goede wrote:
>>>>>>
>>>>>> +&reg_ldo3 {
>>>>>> +       /*
>>>>>> +        * We need to always power the camera sensor, otherwhise all
>>>>>> access
>>>>>> +        * to i2c1 is blocked.
>>>>>> +        */
>>>>>> +       regulator-always-on;
>>>>>> +       regulator-min-microvolt = <2800000>;
>>>>>> +       regulator-max-microvolt = <2800000>;
>>>>>> +       regulator-name = "vdd-csi";
>>>>>> +};
>>>>>
>>>>>
>>>>> What is connected on i2c1 ? Just the camera sensor? or it has some
>>>>> other devices there?
>>>>
>>>>
>>>> The bma250 accelerometer sits there, and the kernel already has a driver
>>>> for it. That driver needs to have devicetree binding support added, and
>>>> then we should be able to use the accelerometer.
>>>
>>>
>>> Ok, so if this regulator is disable, you can't access the other
>>> devices as well, right?
>>
>>
>> Right, the controller reports the bus as being stuck.
>>
>>> Do you know why? Is it the regulator providing
>>> the pull-up voltage?
>>
>>
>> I've tried enabling the pull ups on the SoC i2c pins, so I do not think
>> that it is that, it seems that somehow when not powered the camera sensor is
>> actively keeping the lines low. Either it has multiple power planes, or
>> it is using normally-on fet-s between ground and its i2c lines.
>
> FYI the reference designs use one regulator to power the pull-ups, VCC-PX
> (X for X pin group), and VDDIO (IO power)on the camera sensors. AVDD, DVDD
> (actual power) for the sensors are another (or more) regulators.

But that is a A23 or some such generation thing, right? This is an A10 based
tablet!

Regards,

Hans

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
  2015-09-08 12:49                     ` Hans de Goede
@ 2015-09-08 13:14                         ` Chen-Yu Tsai
  -1 siblings, 0 replies; 44+ messages in thread
From: Chen-Yu Tsai @ 2015-09-08 13:14 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Chen-Yu Tsai, Maxime Ripard, Wolfram Sang, linux-arm-kernel,
	devicetree, linux-sunxi

On Tue, Sep 8, 2015 at 8:49 PM, Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> Hi,
>
>
> On 09/08/2015 10:40 AM, Chen-Yu Tsai wrote:
>>
>> On Tue, Sep 8, 2015 at 3:45 PM, Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>>>
>>> Hi,
>>>
>>> On 07-09-15 22:56, Maxime Ripard wrote:
>>>>
>>>>
>>>> On Mon, Sep 07, 2015 at 09:05:29AM +0200, Hans de Goede wrote:
>>>>>>>
>>>>>>>
>>>>>>> +&reg_ldo3 {
>>>>>>> +       /*
>>>>>>> +        * We need to always power the camera sensor, otherwhise all
>>>>>>> access
>>>>>>> +        * to i2c1 is blocked.
>>>>>>> +        */
>>>>>>> +       regulator-always-on;
>>>>>>> +       regulator-min-microvolt = <2800000>;
>>>>>>> +       regulator-max-microvolt = <2800000>;
>>>>>>> +       regulator-name = "vdd-csi";
>>>>>>> +};
>>>>>>
>>>>>>
>>>>>>
>>>>>> What is connected on i2c1 ? Just the camera sensor? or it has some
>>>>>> other devices there?
>>>>>
>>>>>
>>>>>
>>>>> The bma250 accelerometer sits there, and the kernel already has a
>>>>> driver
>>>>> for it. That driver needs to have devicetree binding support added, and
>>>>> then we should be able to use the accelerometer.
>>>>
>>>>
>>>>
>>>> Ok, so if this regulator is disable, you can't access the other
>>>> devices as well, right?
>>>
>>>
>>>
>>> Right, the controller reports the bus as being stuck.
>>>
>>>> Do you know why? Is it the regulator providing
>>>> the pull-up voltage?
>>>
>>>
>>>
>>> I've tried enabling the pull ups on the SoC i2c pins, so I do not think
>>> that it is that, it seems that somehow when not powered the camera sensor
>>> is
>>> actively keeping the lines low. Either it has multiple power planes, or
>>> it is using normally-on fet-s between ground and its i2c lines.
>>
>>
>> FYI the reference designs use one regulator to power the pull-ups, VCC-PX
>> (X for X pin group), and VDDIO (IO power)on the camera sensors. AVDD, DVDD
>> (actual power) for the sensors are another (or more) regulators.
>
>
> But that is a A23 or some such generation thing, right? This is an A10 based
> tablet!

A13 reference design from

   https://github.com/OLIMEX/OLINUXINO/blob/master/HARDWARE/A13-PDFs/a13-sch.pdf

also has it, but instead of outputs from the AXP223, it's using
discrete regulator
ICs, such as the LP3992 shown in the design. These are likely GPIO enabled.

Looking at some A13 fex files, each camera sensor has a reset GPIO, but the two
share the same power GPIO, which likely enables the regulator.

A10 should be similar, considering they use the same PMIC, and are likely to
need external regulators for all the voltages required.

My point is that the camera sensors require like 2~3 supplies, and VDDIO seems
to be shared between the sensors (if there are 2), the external pull-ups, and
sometimes power to the SoC pin group (if there is a power pin for them).


ChenYu

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [linux-sunxi] Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
@ 2015-09-08 13:14                         ` Chen-Yu Tsai
  0 siblings, 0 replies; 44+ messages in thread
From: Chen-Yu Tsai @ 2015-09-08 13:14 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Sep 8, 2015 at 8:49 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
>
> On 09/08/2015 10:40 AM, Chen-Yu Tsai wrote:
>>
>> On Tue, Sep 8, 2015 at 3:45 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>
>>> Hi,
>>>
>>> On 07-09-15 22:56, Maxime Ripard wrote:
>>>>
>>>>
>>>> On Mon, Sep 07, 2015 at 09:05:29AM +0200, Hans de Goede wrote:
>>>>>>>
>>>>>>>
>>>>>>> +&reg_ldo3 {
>>>>>>> +       /*
>>>>>>> +        * We need to always power the camera sensor, otherwhise all
>>>>>>> access
>>>>>>> +        * to i2c1 is blocked.
>>>>>>> +        */
>>>>>>> +       regulator-always-on;
>>>>>>> +       regulator-min-microvolt = <2800000>;
>>>>>>> +       regulator-max-microvolt = <2800000>;
>>>>>>> +       regulator-name = "vdd-csi";
>>>>>>> +};
>>>>>>
>>>>>>
>>>>>>
>>>>>> What is connected on i2c1 ? Just the camera sensor? or it has some
>>>>>> other devices there?
>>>>>
>>>>>
>>>>>
>>>>> The bma250 accelerometer sits there, and the kernel already has a
>>>>> driver
>>>>> for it. That driver needs to have devicetree binding support added, and
>>>>> then we should be able to use the accelerometer.
>>>>
>>>>
>>>>
>>>> Ok, so if this regulator is disable, you can't access the other
>>>> devices as well, right?
>>>
>>>
>>>
>>> Right, the controller reports the bus as being stuck.
>>>
>>>> Do you know why? Is it the regulator providing
>>>> the pull-up voltage?
>>>
>>>
>>>
>>> I've tried enabling the pull ups on the SoC i2c pins, so I do not think
>>> that it is that, it seems that somehow when not powered the camera sensor
>>> is
>>> actively keeping the lines low. Either it has multiple power planes, or
>>> it is using normally-on fet-s between ground and its i2c lines.
>>
>>
>> FYI the reference designs use one regulator to power the pull-ups, VCC-PX
>> (X for X pin group), and VDDIO (IO power)on the camera sensors. AVDD, DVDD
>> (actual power) for the sensors are another (or more) regulators.
>
>
> But that is a A23 or some such generation thing, right? This is an A10 based
> tablet!

A13 reference design from

   https://github.com/OLIMEX/OLINUXINO/blob/master/HARDWARE/A13-PDFs/a13-sch.pdf

also has it, but instead of outputs from the AXP223, it's using
discrete regulator
ICs, such as the LP3992 shown in the design. These are likely GPIO enabled.

Looking at some A13 fex files, each camera sensor has a reset GPIO, but the two
share the same power GPIO, which likely enables the regulator.

A10 should be similar, considering they use the same PMIC, and are likely to
need external regulators for all the voltages required.

My point is that the camera sensors require like 2~3 supplies, and VDDIO seems
to be shared between the sensors (if there are 2), the external pull-ups, and
sometimes power to the SoC pin group (if there is a power pin for them).


ChenYu

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
  2015-09-08  7:48                             ` [linux-sunxi] " Hans de Goede
@ 2015-09-10 13:56                                 ` Maxime Ripard
  -1 siblings, 0 replies; 44+ messages in thread
From: Maxime Ripard @ 2015-09-10 13:56 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Priit Laes, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw

[-- Attachment #1: Type: text/plain, Size: 2080 bytes --]

On Tue, Sep 08, 2015 at 09:48:48AM +0200, Hans de Goede wrote:
> Hi,
> 
> On 07-09-15 22:52, Maxime Ripard wrote:
> >Hi,
> >
> >On Mon, Sep 07, 2015 at 11:30:03AM +0200, Hans de Goede wrote:
> >>>>>bma250 already has devicetree support. It is used in Gemei G9
> >>>>>tablet (sun4i-gemei-g9.dts).
> >>>>
> >>>>Yes I've seen that, but does it actually work? I've not tried but
> >>>>I do not see any compatible string in the actual bma250 code in
> >>>>the kernel, so I believe that this part of the sun4i-gemei-g9.dts
> >>>>file does not work ?
> >>>
> >>>It worked (even without IRQs) when I submitted the patch. Driver
> >>>itself is under iio/accel/bma180.c
> >>
> >>That is really weird, because:
> >>
> >>https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/iio/accel/bma180.c
> >>
> >>Does not have an of_match_table ... ??
> >
> >Not really, when using DT, i2c will set the i2c_client name to the
> >device part of the compatible [1] [2], and then if the of_device_id
> >lookup fails, will fallback to matching the i2c_client name to the
> >i2c_device_id [3]. Which in our case matches.
> >
> >Maxime
> >
> >[1] http://lxr.free-electrons.com/source/drivers/i2c/i2c-core.c#L1281
> >[2] http://lxr.free-electrons.com/source/drivers/i2c/i2c-core.c#L969
> >[3] http://lxr.free-electrons.com/source/drivers/i2c/i2c-core.c#L461
> 
> Hmm, not sure whether that is a useful feature or an ugly hack :)

Don't shoot the messenger :)

But I don't know either.

> It is sorta unexpected either way. But I can make good use of this to
> enable the accelerometer and maybe also some touchscreens on a bunch of
> boards I have.
> 
> Still should we rely on this? This means relying on Linux kernel behavior,
> rather then something documented in Documentation/devicetree/bindings/...

AFAIK, the behaviour isn't going away, there's been a few patches to
remove the need for i2c_device_id, and they've all been nacked, so I'd
say we can use it as is.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [linux-sunxi] Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
@ 2015-09-10 13:56                                 ` Maxime Ripard
  0 siblings, 0 replies; 44+ messages in thread
From: Maxime Ripard @ 2015-09-10 13:56 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Sep 08, 2015 at 09:48:48AM +0200, Hans de Goede wrote:
> Hi,
> 
> On 07-09-15 22:52, Maxime Ripard wrote:
> >Hi,
> >
> >On Mon, Sep 07, 2015 at 11:30:03AM +0200, Hans de Goede wrote:
> >>>>>bma250 already has devicetree support. It is used in Gemei G9
> >>>>>tablet (sun4i-gemei-g9.dts).
> >>>>
> >>>>Yes I've seen that, but does it actually work? I've not tried but
> >>>>I do not see any compatible string in the actual bma250 code in
> >>>>the kernel, so I believe that this part of the sun4i-gemei-g9.dts
> >>>>file does not work ?
> >>>
> >>>It worked (even without IRQs) when I submitted the patch. Driver
> >>>itself is under iio/accel/bma180.c
> >>
> >>That is really weird, because:
> >>
> >>https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/iio/accel/bma180.c
> >>
> >>Does not have an of_match_table ... ??
> >
> >Not really, when using DT, i2c will set the i2c_client name to the
> >device part of the compatible [1] [2], and then if the of_device_id
> >lookup fails, will fallback to matching the i2c_client name to the
> >i2c_device_id [3]. Which in our case matches.
> >
> >Maxime
> >
> >[1] http://lxr.free-electrons.com/source/drivers/i2c/i2c-core.c#L1281
> >[2] http://lxr.free-electrons.com/source/drivers/i2c/i2c-core.c#L969
> >[3] http://lxr.free-electrons.com/source/drivers/i2c/i2c-core.c#L461
> 
> Hmm, not sure whether that is a useful feature or an ugly hack :)

Don't shoot the messenger :)

But I don't know either.

> It is sorta unexpected either way. But I can make good use of this to
> enable the accelerometer and maybe also some touchscreens on a bunch of
> boards I have.
> 
> Still should we rely on this? This means relying on Linux kernel behavior,
> rather then something documented in Documentation/devicetree/bindings/...

AFAIK, the behaviour isn't going away, there's been a few patches to
remove the need for i2c_device_id, and they've all been nacked, so I'd
say we can use it as is.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150910/dab4bbeb/attachment.sig>

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
  2015-09-08  7:45             ` Hans de Goede
@ 2015-09-13 15:22                 ` Maxime Ripard
  -1 siblings, 0 replies; 44+ messages in thread
From: Maxime Ripard @ 2015-09-13 15:22 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Wolfram Sang, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw

[-- Attachment #1: Type: text/plain, Size: 1936 bytes --]

On Tue, Sep 08, 2015 at 09:45:52AM +0200, Hans de Goede wrote:
> Hi,
> 
> On 07-09-15 22:56, Maxime Ripard wrote:
> >On Mon, Sep 07, 2015 at 09:05:29AM +0200, Hans de Goede wrote:
> >>>>+&reg_ldo3 {
> >>>>+	/*
> >>>>+	 * We need to always power the camera sensor, otherwhise all access
> >>>>+	 * to i2c1 is blocked.
> >>>>+	 */
> >>>>+	regulator-always-on;
> >>>>+	regulator-min-microvolt = <2800000>;
> >>>>+	regulator-max-microvolt = <2800000>;
> >>>>+	regulator-name = "vdd-csi";
> >>>>+};
> >>>
> >>>What is connected on i2c1 ? Just the camera sensor? or it has some
> >>>other devices there?
> >>
> >>The bma250 accelerometer sits there, and the kernel already has a driver
> >>for it. That driver needs to have devicetree binding support added, and
> >>then we should be able to use the accelerometer.
> >
> >Ok, so if this regulator is disable, you can't access the other
> >devices as well, right?
> 
> Right, the controller reports the bus as being stuck.

Which is pretty bad... :/

> >Do you know why? Is it the regulator providing
> >the pull-up voltage?
> 
> I've tried enabling the pull ups on the SoC i2c pins, so I do not think
> that it is that, it seems that somehow when not powered the camera sensor is
> actively keeping the lines low. Either it has multiple power planes, or
> it is using normally-on fet-s between ground and its i2c lines.

Well, if a regulator is powered down, it's also tied to the ground,
which means you would actually have pull-down. Maybe the in-SoC
pullups simply aren't strong enough in such a case.

Anyway. In both cases, the regulator really shouldn't be drifting
along like this. If the i2c bus needs a regulator to be operationaly,
then we can just add an optional bus-supply property or something to
give that to the i2c driver so that it can enable it when needed.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [linux-sunxi] Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
@ 2015-09-13 15:22                 ` Maxime Ripard
  0 siblings, 0 replies; 44+ messages in thread
From: Maxime Ripard @ 2015-09-13 15:22 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Sep 08, 2015 at 09:45:52AM +0200, Hans de Goede wrote:
> Hi,
> 
> On 07-09-15 22:56, Maxime Ripard wrote:
> >On Mon, Sep 07, 2015 at 09:05:29AM +0200, Hans de Goede wrote:
> >>>>+&reg_ldo3 {
> >>>>+	/*
> >>>>+	 * We need to always power the camera sensor, otherwhise all access
> >>>>+	 * to i2c1 is blocked.
> >>>>+	 */
> >>>>+	regulator-always-on;
> >>>>+	regulator-min-microvolt = <2800000>;
> >>>>+	regulator-max-microvolt = <2800000>;
> >>>>+	regulator-name = "vdd-csi";
> >>>>+};
> >>>
> >>>What is connected on i2c1 ? Just the camera sensor? or it has some
> >>>other devices there?
> >>
> >>The bma250 accelerometer sits there, and the kernel already has a driver
> >>for it. That driver needs to have devicetree binding support added, and
> >>then we should be able to use the accelerometer.
> >
> >Ok, so if this regulator is disable, you can't access the other
> >devices as well, right?
> 
> Right, the controller reports the bus as being stuck.

Which is pretty bad... :/

> >Do you know why? Is it the regulator providing
> >the pull-up voltage?
> 
> I've tried enabling the pull ups on the SoC i2c pins, so I do not think
> that it is that, it seems that somehow when not powered the camera sensor is
> actively keeping the lines low. Either it has multiple power planes, or
> it is using normally-on fet-s between ground and its i2c lines.

Well, if a regulator is powered down, it's also tied to the ground,
which means you would actually have pull-down. Maybe the in-SoC
pullups simply aren't strong enough in such a case.

Anyway. In both cases, the regulator really shouldn't be drifting
along like this. If the i2c bus needs a regulator to be operationaly,
then we can just add an optional bus-supply property or something to
give that to the i2c driver so that it can enable it when needed.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150913/99e67cef/attachment.sig>

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
  2015-09-13 15:22                 ` [linux-sunxi] " Maxime Ripard
@ 2015-09-13 17:33                   ` Hans de Goede
  -1 siblings, 0 replies; 44+ messages in thread
From: Hans de Goede @ 2015-09-13 17:33 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Wolfram Sang, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw

Hi,

On 13-09-15 17:22, Maxime Ripard wrote:
> On Tue, Sep 08, 2015 at 09:45:52AM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 07-09-15 22:56, Maxime Ripard wrote:
>>> On Mon, Sep 07, 2015 at 09:05:29AM +0200, Hans de Goede wrote:
>>>>>> +&reg_ldo3 {
>>>>>> +	/*
>>>>>> +	 * We need to always power the camera sensor, otherwhise all access
>>>>>> +	 * to i2c1 is blocked.
>>>>>> +	 */
>>>>>> +	regulator-always-on;
>>>>>> +	regulator-min-microvolt = <2800000>;
>>>>>> +	regulator-max-microvolt = <2800000>;
>>>>>> +	regulator-name = "vdd-csi";
>>>>>> +};
>>>>>
>>>>> What is connected on i2c1 ? Just the camera sensor? or it has some
>>>>> other devices there?
>>>>
>>>> The bma250 accelerometer sits there, and the kernel already has a driver
>>>> for it. That driver needs to have devicetree binding support added, and
>>>> then we should be able to use the accelerometer.
>>>
>>> Ok, so if this regulator is disable, you can't access the other
>>> devices as well, right?
>>
>> Right, the controller reports the bus as being stuck.
>
> Which is pretty bad... :/

Yes.

>>> Do you know why? Is it the regulator providing
>>> the pull-up voltage?
>>
>> I've tried enabling the pull ups on the SoC i2c pins, so I do not think
>> that it is that, it seems that somehow when not powered the camera sensor is
>> actively keeping the lines low. Either it has multiple power planes, or
>> it is using normally-on fet-s between ground and its i2c lines.
>
> Well, if a regulator is powered down, it's also tied to the ground,

? I do not believe that that is necessarily always the case, that would
require an extra fet in the output logic of the pmic which actually
connects it to the ground when powered down. I would expect it to simply
be floating when not enabled.

> which means you would actually have pull-down. Maybe the in-SoC
> pullups simply aren't strong enough in such a case.

This all assumes that that regulator is actually tied to the pull-ups,
something which we've no knowledge of whatsoever.

> Anyway. In both cases, the regulator really shouldn't be drifting
> along like this.

Right which is why I've added the always-on property.

> If the i2c bus needs a regulator to be operationaly,
> then we can just add an optional bus-supply property or something to
> give that to the i2c driver so that it can enable it when needed.

I agree that that would be sensible if this regulator were tied to
the pull-ups, but I've my doubts that it is. We've not seen anything
similar on any other allwinner tablet, other then ChenYu-s Ippo-q8-v5
tablet.

This tablet is sort of a high-end tablet (with a nice ips screen) and
such it also uses a different (better) sensor for its frontcam, a
gc2015 rather then the usual gc0308. I believe that this is the
culprit.

Which would make modelling this as some sort of i2c-bus power-supply
wrong, and I've checked and none of the existing i2c bindings under
Documentation/devicetree/bindings/i2c contain such a thing, so we
would be the first and we will likely have a hard time selling a
binding for this upstream, esp. since we do not know what exactly
is going on.

So all in all I strongly believe that just setting always-on
on the regulator in question is the best solution.

Regards,

Hans





>
> Maxime
>

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [linux-sunxi] Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
@ 2015-09-13 17:33                   ` Hans de Goede
  0 siblings, 0 replies; 44+ messages in thread
From: Hans de Goede @ 2015-09-13 17:33 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 13-09-15 17:22, Maxime Ripard wrote:
> On Tue, Sep 08, 2015 at 09:45:52AM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 07-09-15 22:56, Maxime Ripard wrote:
>>> On Mon, Sep 07, 2015 at 09:05:29AM +0200, Hans de Goede wrote:
>>>>>> +&reg_ldo3 {
>>>>>> +	/*
>>>>>> +	 * We need to always power the camera sensor, otherwhise all access
>>>>>> +	 * to i2c1 is blocked.
>>>>>> +	 */
>>>>>> +	regulator-always-on;
>>>>>> +	regulator-min-microvolt = <2800000>;
>>>>>> +	regulator-max-microvolt = <2800000>;
>>>>>> +	regulator-name = "vdd-csi";
>>>>>> +};
>>>>>
>>>>> What is connected on i2c1 ? Just the camera sensor? or it has some
>>>>> other devices there?
>>>>
>>>> The bma250 accelerometer sits there, and the kernel already has a driver
>>>> for it. That driver needs to have devicetree binding support added, and
>>>> then we should be able to use the accelerometer.
>>>
>>> Ok, so if this regulator is disable, you can't access the other
>>> devices as well, right?
>>
>> Right, the controller reports the bus as being stuck.
>
> Which is pretty bad... :/

Yes.

>>> Do you know why? Is it the regulator providing
>>> the pull-up voltage?
>>
>> I've tried enabling the pull ups on the SoC i2c pins, so I do not think
>> that it is that, it seems that somehow when not powered the camera sensor is
>> actively keeping the lines low. Either it has multiple power planes, or
>> it is using normally-on fet-s between ground and its i2c lines.
>
> Well, if a regulator is powered down, it's also tied to the ground,

? I do not believe that that is necessarily always the case, that would
require an extra fet in the output logic of the pmic which actually
connects it to the ground when powered down. I would expect it to simply
be floating when not enabled.

> which means you would actually have pull-down. Maybe the in-SoC
> pullups simply aren't strong enough in such a case.

This all assumes that that regulator is actually tied to the pull-ups,
something which we've no knowledge of whatsoever.

> Anyway. In both cases, the regulator really shouldn't be drifting
> along like this.

Right which is why I've added the always-on property.

> If the i2c bus needs a regulator to be operationaly,
> then we can just add an optional bus-supply property or something to
> give that to the i2c driver so that it can enable it when needed.

I agree that that would be sensible if this regulator were tied to
the pull-ups, but I've my doubts that it is. We've not seen anything
similar on any other allwinner tablet, other then ChenYu-s Ippo-q8-v5
tablet.

This tablet is sort of a high-end tablet (with a nice ips screen) and
such it also uses a different (better) sensor for its frontcam, a
gc2015 rather then the usual gc0308. I believe that this is the
culprit.

Which would make modelling this as some sort of i2c-bus power-supply
wrong, and I've checked and none of the existing i2c bindings under
Documentation/devicetree/bindings/i2c contain such a thing, so we
would be the first and we will likely have a hard time selling a
binding for this upstream, esp. since we do not know what exactly
is going on.

So all in all I strongly believe that just setting always-on
on the regulator in question is the best solution.

Regards,

Hans





>
> Maxime
>

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
  2015-09-13 17:33                   ` [linux-sunxi] " Hans de Goede
@ 2015-09-22 15:02                       ` Maxime Ripard
  -1 siblings, 0 replies; 44+ messages in thread
From: Maxime Ripard @ 2015-09-22 15:02 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Wolfram Sang, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw

[-- Attachment #1: Type: text/plain, Size: 1964 bytes --]

On Sun, Sep 13, 2015 at 07:33:36PM +0200, Hans de Goede wrote:
> >Anyway. In both cases, the regulator really shouldn't be drifting
> >along like this.
> 
> Right which is why I've added the always-on property.

Which is exactly what I meant by drifting along: that regulator will
never be associated to the i2c bus, and will always be enabled even
though the i2c bus might not even be accessible in the first place
(driver not selected, compiled as a module and not loaded yet), which
is just as bad.

> >If the i2c bus needs a regulator to be operationaly,
> >then we can just add an optional bus-supply property or something to
> >give that to the i2c driver so that it can enable it when needed.
> 
> I agree that that would be sensible if this regulator were tied to
> the pull-ups, but I've my doubts that it is. We've not seen anything
> similar on any other allwinner tablet, other then ChenYu-s Ippo-q8-v5
> tablet.
> 
> This tablet is sort of a high-end tablet (with a nice ips screen) and
> such it also uses a different (better) sensor for its frontcam, a
> gc2015 rather then the usual gc0308. I believe that this is the
> culprit.
> 
> Which would make modelling this as some sort of i2c-bus power-supply
> wrong, and I've checked and none of the existing i2c bindings under
> Documentation/devicetree/bindings/i2c contain such a thing, so we
> would be the first and we will likely have a hard time selling a
> binding for this upstream, esp. since we do not know what exactly
> is going on.

Well, strictly speaking, it is a supply needed to get the bus to
work. We should really try to avoid having always-on for regulators,
especially for devices that are already represented in the DT.

> So all in all I strongly believe that just setting always-on
> on the regulator in question is the best solution.

It's a hack we can avoid.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [linux-sunxi] Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
@ 2015-09-22 15:02                       ` Maxime Ripard
  0 siblings, 0 replies; 44+ messages in thread
From: Maxime Ripard @ 2015-09-22 15:02 UTC (permalink / raw)
  To: linux-arm-kernel

On Sun, Sep 13, 2015 at 07:33:36PM +0200, Hans de Goede wrote:
> >Anyway. In both cases, the regulator really shouldn't be drifting
> >along like this.
> 
> Right which is why I've added the always-on property.

Which is exactly what I meant by drifting along: that regulator will
never be associated to the i2c bus, and will always be enabled even
though the i2c bus might not even be accessible in the first place
(driver not selected, compiled as a module and not loaded yet), which
is just as bad.

> >If the i2c bus needs a regulator to be operationaly,
> >then we can just add an optional bus-supply property or something to
> >give that to the i2c driver so that it can enable it when needed.
> 
> I agree that that would be sensible if this regulator were tied to
> the pull-ups, but I've my doubts that it is. We've not seen anything
> similar on any other allwinner tablet, other then ChenYu-s Ippo-q8-v5
> tablet.
> 
> This tablet is sort of a high-end tablet (with a nice ips screen) and
> such it also uses a different (better) sensor for its frontcam, a
> gc2015 rather then the usual gc0308. I believe that this is the
> culprit.
> 
> Which would make modelling this as some sort of i2c-bus power-supply
> wrong, and I've checked and none of the existing i2c bindings under
> Documentation/devicetree/bindings/i2c contain such a thing, so we
> would be the first and we will likely have a hard time selling a
> binding for this upstream, esp. since we do not know what exactly
> is going on.

Well, strictly speaking, it is a supply needed to get the bus to
work. We should really try to avoid having always-on for regulators,
especially for devices that are already represented in the DT.

> So all in all I strongly believe that just setting always-on
> on the regulator in question is the best solution.

It's a hack we can avoid.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150922/23831324/attachment.sig>

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [linux-sunxi] Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
  2015-09-22 15:02                       ` [linux-sunxi] " Maxime Ripard
@ 2015-09-22 15:24                         ` Hans de Goede
  -1 siblings, 0 replies; 44+ messages in thread
From: Hans de Goede @ 2015-09-22 15:24 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Wolfram Sang, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw

Hi,

On 22-09-15 17:02, Maxime Ripard wrote:
> On Sun, Sep 13, 2015 at 07:33:36PM +0200, Hans de Goede wrote:
>>> Anyway. In both cases, the regulator really shouldn't be drifting
>>> along like this.
>>
>> Right which is why I've added the always-on property.
>
> Which is exactly what I meant by drifting along: that regulator will
> never be associated to the i2c bus, and will always be enabled even
> though the i2c bus might not even be accessible in the first place
> (driver not selected, compiled as a module and not loaded yet), which
> is just as bad.
>
>>> If the i2c bus needs a regulator to be operationaly,
>>> then we can just add an optional bus-supply property or something to
>>> give that to the i2c driver so that it can enable it when needed.
>>
>> I agree that that would be sensible if this regulator were tied to
>> the pull-ups, but I've my doubts that it is. We've not seen anything
>> similar on any other allwinner tablet, other then ChenYu-s Ippo-q8-v5
>> tablet.
>>
>> This tablet is sort of a high-end tablet (with a nice ips screen) and
>> such it also uses a different (better) sensor for its frontcam, a
>> gc2015 rather then the usual gc0308. I believe that this is the
>> culprit.
>>
>> Which would make modelling this as some sort of i2c-bus power-supply
>> wrong, and I've checked and none of the existing i2c bindings under
>> Documentation/devicetree/bindings/i2c contain such a thing, so we
>> would be the first and we will likely have a hard time selling a
>> binding for this upstream, esp. since we do not know what exactly
>> is going on.
>
> Well, strictly speaking, it is a supply needed to get the bus to
> work. We should really try to avoid having always-on for regulators,
> especially for devices that are already represented in the DT.
>
>> So all in all I strongly believe that just setting always-on
>> on the regulator in question is the best solution.
>
> It's a hack we can avoid.

How? By adding a regulator property to the i2c controller node
and then have the i2c controller driver enable this on probe ?

This will make 0 difference in practice since any useful kernel
config will always include the i2c controller as that is necessary
to talk to the pmic which controls this regulator in the first place.

Having some sort of regulator property in the i2c-controller node
might be something worthwhile adding if we knew for sure that that
is how things are wired up, but we simply do not know.

I'm not going to submit a patch to the i2c bindings to add a
regulator if I cannot explain exactly during review why it is
needed.

The way I see it this board has some kinda bug making that bus
not work when the regulator in question is disabled, but we do
not exactly why, so we workaround the bug by not disabling the
regulator -> always-on.

Making some $random-node increase the use-count of the regulator
so that it does not get disabled seems not really helpful since
we do not know where to put the regulator property.

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [linux-sunxi] Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
@ 2015-09-22 15:24                         ` Hans de Goede
  0 siblings, 0 replies; 44+ messages in thread
From: Hans de Goede @ 2015-09-22 15:24 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 22-09-15 17:02, Maxime Ripard wrote:
> On Sun, Sep 13, 2015 at 07:33:36PM +0200, Hans de Goede wrote:
>>> Anyway. In both cases, the regulator really shouldn't be drifting
>>> along like this.
>>
>> Right which is why I've added the always-on property.
>
> Which is exactly what I meant by drifting along: that regulator will
> never be associated to the i2c bus, and will always be enabled even
> though the i2c bus might not even be accessible in the first place
> (driver not selected, compiled as a module and not loaded yet), which
> is just as bad.
>
>>> If the i2c bus needs a regulator to be operationaly,
>>> then we can just add an optional bus-supply property or something to
>>> give that to the i2c driver so that it can enable it when needed.
>>
>> I agree that that would be sensible if this regulator were tied to
>> the pull-ups, but I've my doubts that it is. We've not seen anything
>> similar on any other allwinner tablet, other then ChenYu-s Ippo-q8-v5
>> tablet.
>>
>> This tablet is sort of a high-end tablet (with a nice ips screen) and
>> such it also uses a different (better) sensor for its frontcam, a
>> gc2015 rather then the usual gc0308. I believe that this is the
>> culprit.
>>
>> Which would make modelling this as some sort of i2c-bus power-supply
>> wrong, and I've checked and none of the existing i2c bindings under
>> Documentation/devicetree/bindings/i2c contain such a thing, so we
>> would be the first and we will likely have a hard time selling a
>> binding for this upstream, esp. since we do not know what exactly
>> is going on.
>
> Well, strictly speaking, it is a supply needed to get the bus to
> work. We should really try to avoid having always-on for regulators,
> especially for devices that are already represented in the DT.
>
>> So all in all I strongly believe that just setting always-on
>> on the regulator in question is the best solution.
>
> It's a hack we can avoid.

How? By adding a regulator property to the i2c controller node
and then have the i2c controller driver enable this on probe ?

This will make 0 difference in practice since any useful kernel
config will always include the i2c controller as that is necessary
to talk to the pmic which controls this regulator in the first place.

Having some sort of regulator property in the i2c-controller node
might be something worthwhile adding if we knew for sure that that
is how things are wired up, but we simply do not know.

I'm not going to submit a patch to the i2c bindings to add a
regulator if I cannot explain exactly during review why it is
needed.

The way I see it this board has some kinda bug making that bus
not work when the regulator in question is disabled, but we do
not exactly why, so we workaround the bug by not disabling the
regulator -> always-on.

Making some $random-node increase the use-count of the regulator
so that it does not get disabled seems not really helpful since
we do not know where to put the regulator property.

Regards,

Hans

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [linux-sunxi] Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
  2015-09-22 15:24                         ` Hans de Goede
@ 2015-10-10 12:32                             ` Hans de Goede
  -1 siblings, 0 replies; 44+ messages in thread
From: Hans de Goede @ 2015-10-10 12:32 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Wolfram Sang, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw

Hi Maxime,

I wanted to discuss this with you at ELCE, but I forgot.

I still believe that just setting the regulator to always-on
is the best option, see the end of the quoted mail why I think
that the other options are not a good idea.

I would really like to see this resolved...

On 22-09-15 17:24, Hans de Goede wrote:
> Hi,
>
> On 22-09-15 17:02, Maxime Ripard wrote:
>> On Sun, Sep 13, 2015 at 07:33:36PM +0200, Hans de Goede wrote:
>>>> Anyway. In both cases, the regulator really shouldn't be drifting
>>>> along like this.
>>>
>>> Right which is why I've added the always-on property.
>>
>> Which is exactly what I meant by drifting along: that regulator will
>> never be associated to the i2c bus, and will always be enabled even
>> though the i2c bus might not even be accessible in the first place
>> (driver not selected, compiled as a module and not loaded yet), which
>> is just as bad.
>>
>>>> If the i2c bus needs a regulator to be operationaly,
>>>> then we can just add an optional bus-supply property or something to
>>>> give that to the i2c driver so that it can enable it when needed.
>>>
>>> I agree that that would be sensible if this regulator were tied to
>>> the pull-ups, but I've my doubts that it is. We've not seen anything
>>> similar on any other allwinner tablet, other then ChenYu-s Ippo-q8-v5
>>> tablet.
>>>
>>> This tablet is sort of a high-end tablet (with a nice ips screen) and
>>> such it also uses a different (better) sensor for its frontcam, a
>>> gc2015 rather then the usual gc0308. I believe that this is the
>>> culprit.
>>>
>>> Which would make modelling this as some sort of i2c-bus power-supply
>>> wrong, and I've checked and none of the existing i2c bindings under
>>> Documentation/devicetree/bindings/i2c contain such a thing, so we
>>> would be the first and we will likely have a hard time selling a
>>> binding for this upstream, esp. since we do not know what exactly
>>> is going on.
>>
>> Well, strictly speaking, it is a supply needed to get the bus to
>> work. We should really try to avoid having always-on for regulators,
>> especially for devices that are already represented in the DT.
>>
>>> So all in all I strongly believe that just setting always-on
>>> on the regulator in question is the best solution.
>>
>> It's a hack we can avoid.
>
> How? By adding a regulator property to the i2c controller node
> and then have the i2c controller driver enable this on probe ?
>
> This will make 0 difference in practice since any useful kernel
> config will always include the i2c controller as that is necessary
> to talk to the pmic which controls this regulator in the first place.
>
> Having some sort of regulator property in the i2c-controller node
> might be something worthwhile adding if we knew for sure that that
> is how things are wired up, but we simply do not know.
>
> I'm not going to submit a patch to the i2c bindings to add a
> regulator if I cannot explain exactly during review why it is
> needed.
>
> The way I see it this board has some kinda bug making that bus
> not work when the regulator in question is disabled, but we do
> not exactly why, so we workaround the bug by not disabling the
> regulator -> always-on.
>
> Making some $random-node increase the use-count of the regulator
> so that it does not get disabled seems not really helpful since
> we do not know where to put the regulator property.

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [linux-sunxi] Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
@ 2015-10-10 12:32                             ` Hans de Goede
  0 siblings, 0 replies; 44+ messages in thread
From: Hans de Goede @ 2015-10-10 12:32 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Maxime,

I wanted to discuss this with you at ELCE, but I forgot.

I still believe that just setting the regulator to always-on
is the best option, see the end of the quoted mail why I think
that the other options are not a good idea.

I would really like to see this resolved...

On 22-09-15 17:24, Hans de Goede wrote:
> Hi,
>
> On 22-09-15 17:02, Maxime Ripard wrote:
>> On Sun, Sep 13, 2015 at 07:33:36PM +0200, Hans de Goede wrote:
>>>> Anyway. In both cases, the regulator really shouldn't be drifting
>>>> along like this.
>>>
>>> Right which is why I've added the always-on property.
>>
>> Which is exactly what I meant by drifting along: that regulator will
>> never be associated to the i2c bus, and will always be enabled even
>> though the i2c bus might not even be accessible in the first place
>> (driver not selected, compiled as a module and not loaded yet), which
>> is just as bad.
>>
>>>> If the i2c bus needs a regulator to be operationaly,
>>>> then we can just add an optional bus-supply property or something to
>>>> give that to the i2c driver so that it can enable it when needed.
>>>
>>> I agree that that would be sensible if this regulator were tied to
>>> the pull-ups, but I've my doubts that it is. We've not seen anything
>>> similar on any other allwinner tablet, other then ChenYu-s Ippo-q8-v5
>>> tablet.
>>>
>>> This tablet is sort of a high-end tablet (with a nice ips screen) and
>>> such it also uses a different (better) sensor for its frontcam, a
>>> gc2015 rather then the usual gc0308. I believe that this is the
>>> culprit.
>>>
>>> Which would make modelling this as some sort of i2c-bus power-supply
>>> wrong, and I've checked and none of the existing i2c bindings under
>>> Documentation/devicetree/bindings/i2c contain such a thing, so we
>>> would be the first and we will likely have a hard time selling a
>>> binding for this upstream, esp. since we do not know what exactly
>>> is going on.
>>
>> Well, strictly speaking, it is a supply needed to get the bus to
>> work. We should really try to avoid having always-on for regulators,
>> especially for devices that are already represented in the DT.
>>
>>> So all in all I strongly believe that just setting always-on
>>> on the regulator in question is the best solution.
>>
>> It's a hack we can avoid.
>
> How? By adding a regulator property to the i2c controller node
> and then have the i2c controller driver enable this on probe ?
>
> This will make 0 difference in practice since any useful kernel
> config will always include the i2c controller as that is necessary
> to talk to the pmic which controls this regulator in the first place.
>
> Having some sort of regulator property in the i2c-controller node
> might be something worthwhile adding if we knew for sure that that
> is how things are wired up, but we simply do not know.
>
> I'm not going to submit a patch to the i2c bindings to add a
> regulator if I cannot explain exactly during review why it is
> needed.
>
> The way I see it this board has some kinda bug making that bus
> not work when the regulator in question is disabled, but we do
> not exactly why, so we workaround the bug by not disabling the
> regulator -> always-on.
>
> Making some $random-node increase the use-count of the regulator
> so that it does not get disabled seems not really helpful since
> we do not know where to put the regulator property.

Regards,

Hans

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
  2015-09-22 15:24                         ` Hans de Goede
@ 2015-10-19 19:43                             ` Maxime Ripard
  -1 siblings, 0 replies; 44+ messages in thread
From: Maxime Ripard @ 2015-10-19 19:43 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Wolfram Sang, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw

[-- Attachment #1: Type: text/plain, Size: 4022 bytes --]

On Tue, Sep 22, 2015 at 05:24:37PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 22-09-15 17:02, Maxime Ripard wrote:
> >On Sun, Sep 13, 2015 at 07:33:36PM +0200, Hans de Goede wrote:
> >>>Anyway. In both cases, the regulator really shouldn't be drifting
> >>>along like this.
> >>
> >>Right which is why I've added the always-on property.
> >
> >Which is exactly what I meant by drifting along: that regulator will
> >never be associated to the i2c bus, and will always be enabled even
> >though the i2c bus might not even be accessible in the first place
> >(driver not selected, compiled as a module and not loaded yet), which
> >is just as bad.
> >
> >>>If the i2c bus needs a regulator to be operationaly,
> >>>then we can just add an optional bus-supply property or something to
> >>>give that to the i2c driver so that it can enable it when needed.
> >>
> >>I agree that that would be sensible if this regulator were tied to
> >>the pull-ups, but I've my doubts that it is. We've not seen anything
> >>similar on any other allwinner tablet, other then ChenYu-s Ippo-q8-v5
> >>tablet.
> >>
> >>This tablet is sort of a high-end tablet (with a nice ips screen) and
> >>such it also uses a different (better) sensor for its frontcam, a
> >>gc2015 rather then the usual gc0308. I believe that this is the
> >>culprit.
> >>
> >>Which would make modelling this as some sort of i2c-bus power-supply
> >>wrong, and I've checked and none of the existing i2c bindings under
> >>Documentation/devicetree/bindings/i2c contain such a thing, so we
> >>would be the first and we will likely have a hard time selling a
> >>binding for this upstream, esp. since we do not know what exactly
> >>is going on.
> >
> >Well, strictly speaking, it is a supply needed to get the bus to
> >work. We should really try to avoid having always-on for regulators,
> >especially for devices that are already represented in the DT.
> >
> >>So all in all I strongly believe that just setting always-on
> >>on the regulator in question is the best solution.
> >
> >It's a hack we can avoid.
> 
> How? By adding a regulator property to the i2c controller node
> and then have the i2c controller driver enable this on probe ?

Yes.

> This will make 0 difference in practice since any useful kernel
> config will always include the i2c controller as that is necessary
> to talk to the pmic which controls this regulator in the first place.

It's a choice we don't enforce. We had the option to run the kernel
without a PMIC for a long time, and it's still an option. Not the best
one, but an option.

And there's also the case where the user doesn't want to load or wants
to unload the module. It's also a valid case.

> Having some sort of regulator property in the i2c-controller node
> might be something worthwhile adding if we knew for sure that that
> is how things are wired up, but we simply do not know.

We have something called the device model, and we built most
frameworks around it. Your patch simply circumvents it, and while we
have a nice way to tie a resource to a device, you're not using it at
all.

This has the nasty side effect of not being able to control your
resources, for example to shutdown the regulator when the bus is no
longer in use. I remember having kind of the same argument with
simplefb, but with you on the opposite side of the argument :)

It also avoids debugging things properly, since the regulator isn't
linked to the device actually using it.

> I'm not going to submit a patch to the i2c bindings to add a
> regulator if I cannot explain exactly during review why it is
> needed.

And I'm not going to pull something like that. Sorry.

> The way I see it this board has some kinda bug making that bus
> not work when the regulator in question is disabled, but we do
> not exactly why, so we workaround the bug by not disabling the
> regulator -> always-on.

Yes. As long as the bus is in use.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [linux-sunxi] Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
@ 2015-10-19 19:43                             ` Maxime Ripard
  0 siblings, 0 replies; 44+ messages in thread
From: Maxime Ripard @ 2015-10-19 19:43 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Sep 22, 2015 at 05:24:37PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 22-09-15 17:02, Maxime Ripard wrote:
> >On Sun, Sep 13, 2015 at 07:33:36PM +0200, Hans de Goede wrote:
> >>>Anyway. In both cases, the regulator really shouldn't be drifting
> >>>along like this.
> >>
> >>Right which is why I've added the always-on property.
> >
> >Which is exactly what I meant by drifting along: that regulator will
> >never be associated to the i2c bus, and will always be enabled even
> >though the i2c bus might not even be accessible in the first place
> >(driver not selected, compiled as a module and not loaded yet), which
> >is just as bad.
> >
> >>>If the i2c bus needs a regulator to be operationaly,
> >>>then we can just add an optional bus-supply property or something to
> >>>give that to the i2c driver so that it can enable it when needed.
> >>
> >>I agree that that would be sensible if this regulator were tied to
> >>the pull-ups, but I've my doubts that it is. We've not seen anything
> >>similar on any other allwinner tablet, other then ChenYu-s Ippo-q8-v5
> >>tablet.
> >>
> >>This tablet is sort of a high-end tablet (with a nice ips screen) and
> >>such it also uses a different (better) sensor for its frontcam, a
> >>gc2015 rather then the usual gc0308. I believe that this is the
> >>culprit.
> >>
> >>Which would make modelling this as some sort of i2c-bus power-supply
> >>wrong, and I've checked and none of the existing i2c bindings under
> >>Documentation/devicetree/bindings/i2c contain such a thing, so we
> >>would be the first and we will likely have a hard time selling a
> >>binding for this upstream, esp. since we do not know what exactly
> >>is going on.
> >
> >Well, strictly speaking, it is a supply needed to get the bus to
> >work. We should really try to avoid having always-on for regulators,
> >especially for devices that are already represented in the DT.
> >
> >>So all in all I strongly believe that just setting always-on
> >>on the regulator in question is the best solution.
> >
> >It's a hack we can avoid.
> 
> How? By adding a regulator property to the i2c controller node
> and then have the i2c controller driver enable this on probe ?

Yes.

> This will make 0 difference in practice since any useful kernel
> config will always include the i2c controller as that is necessary
> to talk to the pmic which controls this regulator in the first place.

It's a choice we don't enforce. We had the option to run the kernel
without a PMIC for a long time, and it's still an option. Not the best
one, but an option.

And there's also the case where the user doesn't want to load or wants
to unload the module. It's also a valid case.

> Having some sort of regulator property in the i2c-controller node
> might be something worthwhile adding if we knew for sure that that
> is how things are wired up, but we simply do not know.

We have something called the device model, and we built most
frameworks around it. Your patch simply circumvents it, and while we
have a nice way to tie a resource to a device, you're not using it at
all.

This has the nasty side effect of not being able to control your
resources, for example to shutdown the regulator when the bus is no
longer in use. I remember having kind of the same argument with
simplefb, but with you on the opposite side of the argument :)

It also avoids debugging things properly, since the regulator isn't
linked to the device actually using it.

> I'm not going to submit a patch to the i2c bindings to add a
> regulator if I cannot explain exactly during review why it is
> needed.

And I'm not going to pull something like that. Sorry.

> The way I see it this board has some kinda bug making that bus
> not work when the regulator in question is disabled, but we do
> not exactly why, so we workaround the bug by not disabling the
> regulator -> always-on.

Yes. As long as the bus is in use.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151019/dcf3c1b6/attachment.sig>

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
  2015-10-19 19:43                             ` [linux-sunxi] " Maxime Ripard
@ 2015-10-20 21:59                               ` Hans de Goede
  -1 siblings, 0 replies; 44+ messages in thread
From: Hans de Goede @ 2015-10-20 21:59 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Wolfram Sang, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw

Hi,

On 10/19/2015 09:43 PM, Maxime Ripard wrote:
> On Tue, Sep 22, 2015 at 05:24:37PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 22-09-15 17:02, Maxime Ripard wrote:
>>> On Sun, Sep 13, 2015 at 07:33:36PM +0200, Hans de Goede wrote:
>>>>> Anyway. In both cases, the regulator really shouldn't be drifting
>>>>> along like this.
>>>>
>>>> Right which is why I've added the always-on property.
>>>
>>> Which is exactly what I meant by drifting along: that regulator will
>>> never be associated to the i2c bus, and will always be enabled even
>>> though the i2c bus might not even be accessible in the first place
>>> (driver not selected, compiled as a module and not loaded yet), which
>>> is just as bad.
>>>
>>>>> If the i2c bus needs a regulator to be operationaly,
>>>>> then we can just add an optional bus-supply property or something to
>>>>> give that to the i2c driver so that it can enable it when needed.
>>>>
>>>> I agree that that would be sensible if this regulator were tied to
>>>> the pull-ups, but I've my doubts that it is. We've not seen anything
>>>> similar on any other allwinner tablet, other then ChenYu-s Ippo-q8-v5
>>>> tablet.
>>>>
>>>> This tablet is sort of a high-end tablet (with a nice ips screen) and
>>>> such it also uses a different (better) sensor for its frontcam, a
>>>> gc2015 rather then the usual gc0308. I believe that this is the
>>>> culprit.
>>>>
>>>> Which would make modelling this as some sort of i2c-bus power-supply
>>>> wrong, and I've checked and none of the existing i2c bindings under
>>>> Documentation/devicetree/bindings/i2c contain such a thing, so we
>>>> would be the first and we will likely have a hard time selling a
>>>> binding for this upstream, esp. since we do not know what exactly
>>>> is going on.
>>>
>>> Well, strictly speaking, it is a supply needed to get the bus to
>>> work. We should really try to avoid having always-on for regulators,
>>> especially for devices that are already represented in the DT.
>>>
>>>> So all in all I strongly believe that just setting always-on
>>>> on the regulator in question is the best solution.
>>>
>>> It's a hack we can avoid.
>>
>> How? By adding a regulator property to the i2c controller node
>> and then have the i2c controller driver enable this on probe ?
>
> Yes.

Ok, I'll send a v2 marking i2c1 as failed for now, as you've done with
sun6i-a31-hummingbird.dts, and drop the ldo3 node.

And I'll put looking into fixing this via a supply property for the
i2c-host node on my todo list.

Regards,

Hans

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [linux-sunxi] Re: [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet
@ 2015-10-20 21:59                               ` Hans de Goede
  0 siblings, 0 replies; 44+ messages in thread
From: Hans de Goede @ 2015-10-20 21:59 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 10/19/2015 09:43 PM, Maxime Ripard wrote:
> On Tue, Sep 22, 2015 at 05:24:37PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 22-09-15 17:02, Maxime Ripard wrote:
>>> On Sun, Sep 13, 2015 at 07:33:36PM +0200, Hans de Goede wrote:
>>>>> Anyway. In both cases, the regulator really shouldn't be drifting
>>>>> along like this.
>>>>
>>>> Right which is why I've added the always-on property.
>>>
>>> Which is exactly what I meant by drifting along: that regulator will
>>> never be associated to the i2c bus, and will always be enabled even
>>> though the i2c bus might not even be accessible in the first place
>>> (driver not selected, compiled as a module and not loaded yet), which
>>> is just as bad.
>>>
>>>>> If the i2c bus needs a regulator to be operationaly,
>>>>> then we can just add an optional bus-supply property or something to
>>>>> give that to the i2c driver so that it can enable it when needed.
>>>>
>>>> I agree that that would be sensible if this regulator were tied to
>>>> the pull-ups, but I've my doubts that it is. We've not seen anything
>>>> similar on any other allwinner tablet, other then ChenYu-s Ippo-q8-v5
>>>> tablet.
>>>>
>>>> This tablet is sort of a high-end tablet (with a nice ips screen) and
>>>> such it also uses a different (better) sensor for its frontcam, a
>>>> gc2015 rather then the usual gc0308. I believe that this is the
>>>> culprit.
>>>>
>>>> Which would make modelling this as some sort of i2c-bus power-supply
>>>> wrong, and I've checked and none of the existing i2c bindings under
>>>> Documentation/devicetree/bindings/i2c contain such a thing, so we
>>>> would be the first and we will likely have a hard time selling a
>>>> binding for this upstream, esp. since we do not know what exactly
>>>> is going on.
>>>
>>> Well, strictly speaking, it is a supply needed to get the bus to
>>> work. We should really try to avoid having always-on for regulators,
>>> especially for devices that are already represented in the DT.
>>>
>>>> So all in all I strongly believe that just setting always-on
>>>> on the regulator in question is the best solution.
>>>
>>> It's a hack we can avoid.
>>
>> How? By adding a regulator property to the i2c controller node
>> and then have the i2c controller driver enable this on probe ?
>
> Yes.

Ok, I'll send a v2 marking i2c1 as failed for now, as you've done with
sun6i-a31-hummingbird.dts, and drop the ldo3 node.

And I'll put looking into fixing this via a supply property for the
i2c-host node on my todo list.

Regards,

Hans

^ permalink raw reply	[flat|nested] 44+ messages in thread

end of thread, other threads:[~2015-10-20 21:59 UTC | newest]

Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-05  8:21 [PATCH] ARM: dts: sun4i: Add dts file for the pov protab2-ips9 tablet Hans de Goede
2015-09-05  8:21 ` Hans de Goede
     [not found] ` <1441441319-10658-1-git-send-email-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2015-09-06 16:30   ` Maxime Ripard
2015-09-06 16:30     ` Maxime Ripard
2015-09-07  7:05     ` Hans de Goede
2015-09-07  7:05       ` [linux-sunxi] " Hans de Goede
     [not found]       ` <55ED3739.609-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2015-09-07  7:49         ` Priit Laes
2015-09-07  7:49           ` [linux-sunxi] " Priit Laes
     [not found]           ` <1441612142.14422.34.camel-q/aMd4JkU83YtjvyW6yDsg@public.gmane.org>
2015-09-07  8:49             ` Hans de Goede
2015-09-07  8:49               ` Hans de Goede
     [not found]               ` <55ED4F8F.6080502-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2015-09-07  9:14                 ` Priit Laes
2015-09-07  9:14                   ` Priit Laes
     [not found]                   ` <1441617262.21530.4.camel-q/aMd4JkU83YtjvyW6yDsg@public.gmane.org>
2015-09-07  9:30                     ` Hans de Goede
2015-09-07  9:30                       ` Hans de Goede
     [not found]                       ` <55ED591B.7050705-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2015-09-07 20:52                         ` Maxime Ripard
2015-09-07 20:52                           ` [linux-sunxi] " Maxime Ripard
2015-09-08  7:48                           ` Hans de Goede
2015-09-08  7:48                             ` [linux-sunxi] " Hans de Goede
     [not found]                             ` <55EE92E0.1010805-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2015-09-10 13:56                               ` Maxime Ripard
2015-09-10 13:56                                 ` [linux-sunxi] " Maxime Ripard
2015-09-07 20:56         ` Maxime Ripard
2015-09-07 20:56           ` [linux-sunxi] " Maxime Ripard
2015-09-08  7:45           ` Hans de Goede
2015-09-08  7:45             ` Hans de Goede
     [not found]             ` <55EE9230.70209-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2015-09-08  8:40               ` Chen-Yu Tsai
2015-09-08  8:40                 ` [linux-sunxi] " Chen-Yu Tsai
     [not found]                 ` <CAGb2v64BGPP9Fs0gM0ghJZ0tO8QdGZD9GfpquGCGKiZCeTkyww-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-09-08 12:49                   ` Hans de Goede
2015-09-08 12:49                     ` Hans de Goede
     [not found]                     ` <55EED95D.20004-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2015-09-08 13:14                       ` Chen-Yu Tsai
2015-09-08 13:14                         ` [linux-sunxi] " Chen-Yu Tsai
2015-09-13 15:22               ` Maxime Ripard
2015-09-13 15:22                 ` [linux-sunxi] " Maxime Ripard
2015-09-13 17:33                 ` Hans de Goede
2015-09-13 17:33                   ` [linux-sunxi] " Hans de Goede
     [not found]                   ` <55F5B370.7040203-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2015-09-22 15:02                     ` Maxime Ripard
2015-09-22 15:02                       ` [linux-sunxi] " Maxime Ripard
2015-09-22 15:24                       ` Hans de Goede
2015-09-22 15:24                         ` Hans de Goede
     [not found]                         ` <560172B5.8090709-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2015-10-10 12:32                           ` Hans de Goede
2015-10-10 12:32                             ` Hans de Goede
2015-10-19 19:43                           ` Maxime Ripard
2015-10-19 19:43                             ` [linux-sunxi] " Maxime Ripard
2015-10-20 21:59                             ` Hans de Goede
2015-10-20 21:59                               ` [linux-sunxi] " Hans de Goede

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.