Re: [PATCH v2 7/7] ARM: dts: sun4i: Add touchscreen node to pov protab2-ips9 tablet
On Fri, Nov 20, 2015 at 02:24:52PM +0100, Hans de Goede wrote: > Add a node describing the touchscreen found on the pov protab2-ips9 > tablet. > > Signed-off-by: Hans de Goede Applied 6 and 7, thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH 3/4] input: Add new sun4i-keypad driver
Hi Yassin, On Fri, Sep 18, 2015 at 10:19:55AM +1000, Yassin Jaffer wrote: > Hi Maxime > > I appreciate your time and efforts . > > Do you need that rate to be enforced, or is it some leftover from the > > allwinner BSP? > > > I've found that clock rate works fine with the default denounce and scan > cycle. It was not really my point. My point was do you *need* it to operate. And so your properties were in clock cycles? Please use a unit indenpendant of the clock rate, like seconds or Hz (or any multiple of them). > By the way do you have any dev board which expose the keypad pins? I don't think I have any. Thanks, Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH 3/4] input: Add new sun4i-keypad driver
t(dev, "apb");
> + if (IS_ERR(keypad->apb_clk)) {
> + dev_err(dev, "failed to get a apb clock.\n");
> + return PTR_ERR(keypad->apb_clk);
> + }
> +
> + keypad->clk = devm_clk_get(dev, "keypad");
> + if (IS_ERR(keypad->clk)) {
> + dev_err(dev, "failed to get a keypad clock.\n");
> + return PTR_ERR(keypad->clk);
> + }
> +
> + ret = clk_set_rate(keypad->clk, KP_BASE_CLK);
> + if (ret) {
> + dev_err(dev, "set keypad base clock failed!\n");
> + return ret;
> + }
Do you need that rate to be enforced, or is it some leftover from the
allwinner BSP?
> + ret = input_register_device(keypad->input);
> + if (ret)
> + return ret;
> +
> + platform_set_drvdata(pdev, keypad);
> +
> + return 0;
> +}
> +
> +static int sun4i_keypad_remove(struct platform_device *pdev)
> +{
> + struct sun4i_keypad_data *keypad = platform_get_drvdata(pdev);
> +
> + free_irq(keypad->irq, keypad);
> + input_unregister_device(keypad->input);
> + kfree(keypad);
The point of using devm_ functions is precisely to remove the removal
code from both the probe error path (and you did so there), and from
the remove.
> +
> + return 0;
> +}
> +
> +static const struct of_device_id sun4i_keypad_of_match[] = {
> + { .compatible = "allwinner,sun4i-a10-keypad", },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, sun4i_keypad_of_match);
> +
> +static struct platform_driver sun4i_keypad_driver = {
> + .driver = {
> + .name = "sun4i-a10-keypad",
> + .of_match_table = of_match_ptr(sun4i_keypad_of_match),
> + },
> + .probe = sun4i_keypad_probe,
> + .remove = sun4i_keypad_remove,
> +};
> +
> +module_platform_driver(sun4i_keypad_driver);
> +
> +MODULE_DESCRIPTION("Allwinner sun4i keypad controller driver");
> +MODULE_AUTHOR("Yassin Jaffer ");
> +MODULE_LICENSE("GPL");
> +
It looks good otherwise, thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH 2/4] ARM: dts: sun7i: Add keypad node to Allwinner A20 SoC
Hi Yassin,
On Wed, Sep 16, 2015 at 12:05:55AM +1000, yassinjaf...@gmail.com wrote:
> From: Yassin Jaffer
>
> Add Keypad controller node definition to the A20 SoC.
>
> Signed-off-by: Yassin Jaffer
> ---
> arch/arm/boot/dts/sun7i-a20.dtsi | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi
> b/arch/arm/boot/dts/sun7i-a20.dtsi
> index 333604a..35cc8d0 100644
> --- a/arch/arm/boot/dts/sun7i-a20.dtsi
> +++ b/arch/arm/boot/dts/sun7i-a20.dtsi
> @@ -1198,6 +1198,15 @@
> status = "disabled";
> };
>
> + kp: kp@01c23000 {
The node name should reflect the class of the device. keypad@01c23000
would be better for example.
It looks good otherwise.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH 1/4] ARM:dts:sun7i: Add keypad clk node
On Wed, Sep 16, 2015 at 12:05:54AM +1000, yassinjaf...@gmail.com wrote: > From: Yassin Jaffer > > This patch add support to the keypad clock on sun7i > > Signed-off-by: Yassin Jaffer Applied, thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH 4/4] devicetree: bindings:Allwinner sun4i keypad
On Wed, Sep 16, 2015 at 12:05:57AM +1000, yassinjaf...@gmail.com wrote:
> From: Yassin Jaffer
>
> Signed-off-by: Yassin Jaffer
> ---
> .../devicetree/bindings/input/sun4i-keypad.txt | 56
> ++
> 1 file changed, 56 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/sun4i-keypad.txt
>
> diff --git a/Documentation/devicetree/bindings/input/sun4i-keypad.txt
> b/Documentation/devicetree/bindings/input/sun4i-keypad.txt
> new file mode 100644
> index 000..60ed0f2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/sun4i-keypad.txt
> @@ -0,0 +1,56 @@
> +Allwinner sun4i keypad
> +
> +
> +Required properties:
> + - compatible: "allwinner,sun4i-a10-keypad"
> + - reg: mmio address range of the chip
> + - interrupts: interrupt to which the chip is connected
> + - clocks : shall reference keypad controller clocks.
> + - clock-names : keypad controller internal clock names. Shall contain :
> +* "apb" : APB gating clock
> +* "keypad" : keypad controller clock
> +
> +Required Board Specific Properties:
> +- pinctrl-names: The definition can be found at
> +pinctrl/pinctrl-bindings.txt.
> +
> +- pinctrl-0: The definition can be found at
> +pinctrl/pinctrl-bindings.txt.
> +
> +- linux,keymap: The definition can be found at
> +bindings/input/matrix-keymap.txt.
> +
> +Optional properties:
> + - scan-cycle: device specific scan cycle
> + - debounce-cycle: device specific debounce cycle
> + - autorepeat: If specified device will autorepeat
Are those properties generic? I couldn't find them defined anywhere.
In which units are those properties?
> +
> +Example:
> +
> +#include
> +
> + kp: kp@01c23000 {
> + compatible = "allwinner,sun4i-a10-keypad";
> + reg = <0x01c23000 0x400>;
> + interrupts = ;
> + clocks = <&apb0_gates 10>, <&keypad_clk>;
> + clock-names = "apb", "keypad";
> + pinctrl-names = "default";
> + pinctrl-0 = <&keypad_rows>, <&keypad_cols>;
> + linux,keymap = <0x0067 /* KEY_UP */
> + 0x0001006c /* KEY_DOWN */
> + 0x00020072 /* KEY_VOLUMEDOWN */
> + 0x00030066 /* KEY_HOME */
> + 0x016a /* KEY_RIGHT */
> + 0x01010069 /* KEY_LEFT */
> + 0x0102001c /* KEY_ENTER */
> + 0x01030073 /* KEY_VOLUMEUP */
> + 0x0240 /* KEY_F6 */
> + 0x02010042 /* KEY_F8 */
> + 0x02020043 /* KEY_F9 */
> + 0x02030044 /* KEY_F10 */
> + 0x033b /* KEY_F1 */
> + 0x0301003c /* KEY_F2 */
> + 0x0302003d /* KEY_F3 */
> + 0x03030074>;/* KEY_POWER */
You don't seem to use the header you just told us to include.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH] touchscreen: sun4i-ts: Really fix A10 temperature reporting
Hi, On Wed, Jun 24, 2015 at 09:16:54AM +0200, Hans de Goede wrote: > Maxime I think we should send the mentioned dts patch to stable for > 4.1 , do you agree ? Yep. We should definitely do that. You take care of this? Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v3 0/4] input: ft5x06: Fix userspace reported maximum value
On Tue, Mar 24, 2015 at 11:56:14AM -0700, Dmitry Torokhov wrote: > On Tue, Mar 24, 2015 at 11:45:20AM -0700, Maxime Ripard wrote: > > On Tue, Mar 24, 2015 at 09:47:22AM -0700, Dmitry Torokhov wrote: > > > On Mon, Mar 23, 2015 at 05:10:02PM -0700, Maxime Ripard wrote: > > > > Hi Lothar, Dmitry, > > > > > > > > On Tue, Mar 03, 2015 at 02:50:42PM +0100, Maxime Ripard wrote: > > > > > Hi, > > > > > > > > > > The current ft5x06 reports to the user-space that its maximum > > > > > coordinates are, on both X and Y, way higher than what could be > > > > > actually usable on the screen (in my case, 5759x1151 instead of > > > > > 480x800). > > > > > > > > > > This causes trouble on some userspace stacks that then try to re-scale > > > > > these coordinates back to the framebuffer resolution, like QT does. > > > > > > > > > > Use the of_touchscreen code to find the real touchscreen limits in the > > > > > DT case, and report that to the userspace. > > > > > > > > Do you have any comments on that? > > > > > > Yes, I'll apply it, although if you could supply the updated patch > > > description for #1 that would be great. The current one seems from the > > > old version and is no longer accurate. > > > > > > Thanks. > > > > You're right. > > > > Would something like: > > > > """ > > input: touchscreen: of: Rework the DT parsing function > > > > The DT parsing function currently duplicates a lot of the code to > > parse the touchscreen DT properties. > > > > In order to ease further additions to this parsing routine, rework it > > slightly to create new helper functions. > > > > Signed-off-by: Maxime Ripard > > """ > > > > be good enough? > > Yep, thanks, I'll update the description, consider applied. > > Thanks. Great, thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v3 0/4] input: ft5x06: Fix userspace reported maximum value
On Tue, Mar 24, 2015 at 09:47:22AM -0700, Dmitry Torokhov wrote: > On Mon, Mar 23, 2015 at 05:10:02PM -0700, Maxime Ripard wrote: > > Hi Lothar, Dmitry, > > > > On Tue, Mar 03, 2015 at 02:50:42PM +0100, Maxime Ripard wrote: > > > Hi, > > > > > > The current ft5x06 reports to the user-space that its maximum > > > coordinates are, on both X and Y, way higher than what could be > > > actually usable on the screen (in my case, 5759x1151 instead of > > > 480x800). > > > > > > This causes trouble on some userspace stacks that then try to re-scale > > > these coordinates back to the framebuffer resolution, like QT does. > > > > > > Use the of_touchscreen code to find the real touchscreen limits in the > > > DT case, and report that to the userspace. > > > > Do you have any comments on that? > > Yes, I'll apply it, although if you could supply the updated patch > description for #1 that would be great. The current one seems from the > old version and is no longer accurate. > > Thanks. You're right. Would something like: """ input: touchscreen: of: Rework the DT parsing function The DT parsing function currently duplicates a lot of the code to parse the touchscreen DT properties. In order to ease further additions to this parsing routine, rework it slightly to create new helper functions. Signed-off-by: Maxime Ripard """ be good enough? Thanks, Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v3 0/4] input: ft5x06: Fix userspace reported maximum value
Hi Lothar, Dmitry, On Tue, Mar 03, 2015 at 02:50:42PM +0100, Maxime Ripard wrote: > Hi, > > The current ft5x06 reports to the user-space that its maximum > coordinates are, on both X and Y, way higher than what could be > actually usable on the screen (in my case, 5759x1151 instead of > 480x800). > > This causes trouble on some userspace stacks that then try to re-scale > these coordinates back to the framebuffer resolution, like QT does. > > Use the of_touchscreen code to find the real touchscreen limits in the > DT case, and report that to the userspace. Do you have any comments on that? Thanks, Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
[PATCH v3 4/4] input: edt-ft5x06: Remove EV_SYN event report
input_register_device already sets the EV_SYN event since all devices can generate them. Remove the redundant affectation. Signed-off-by: Maxime Ripard --- drivers/input/touchscreen/edt-ft5x06.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c index cd8a7472266b..e6aef3e48bd9 100644 --- a/drivers/input/touchscreen/edt-ft5x06.c +++ b/drivers/input/touchscreen/edt-ft5x06.c @@ -1035,7 +1035,6 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client, input->id.bustype = BUS_I2C; input->dev.parent = &client->dev; - __set_bit(EV_SYN, input->evbit); __set_bit(EV_KEY, input->evbit); __set_bit(EV_ABS, input->evbit); __set_bit(BTN_TOUCH, input->keybit); -- 2.3.0 -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/4] input: touchscreen: of: Register multitouch axes
So far, the DT parsing code was only setting up the regular input axes,
completely ignoring their multitouch counter parts.
Fill them with the same parameters than the regular axes.
Signed-off-by: Maxime Ripard
---
drivers/input/touchscreen/of_touchscreen.c | 22 +-
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/drivers/input/touchscreen/of_touchscreen.c
b/drivers/input/touchscreen/of_touchscreen.c
index fe80d8ba7efa..b82b5207c78b 100644
--- a/drivers/input/touchscreen/of_touchscreen.c
+++ b/drivers/input/touchscreen/of_touchscreen.c
@@ -11,6 +11,7 @@
#include
#include
+#include
#include
static u32 of_get_optional_u32(struct device_node *np,
@@ -30,8 +31,13 @@ static void touchscreen_set_params(struct input_dev *dev,
struct input_absinfo *absinfo;
if (!test_bit(axis, dev->absbit)) {
- dev_warn(&dev->dev,
-"DT specifies parameters but the axis is not set
up\n");
+ /*
+* Emit a warning only if the axis is not a multitouch
+* axis, which might not be set by the driver.
+*/
+ if (!input_is_mt_axis(axis))
+ dev_warn(&dev->dev,
+"DT specifies parameters but the axis is not
set up\n");
return;
}
@@ -59,17 +65,23 @@ void touchscreen_parse_of_params(struct input_dev *dev)
maximum = of_get_optional_u32(np, "touchscreen-size-x");
fuzz = of_get_optional_u32(np, "touchscreen-fuzz-x");
- if (maximum || fuzz)
+ if (maximum || fuzz) {
touchscreen_set_params(dev, ABS_X, maximum, fuzz);
+ touchscreen_set_params(dev, ABS_MT_POSITION_X, maximum, fuzz);
+ }
maximum = of_get_optional_u32(np, "touchscreen-size-y");
fuzz = of_get_optional_u32(np, "touchscreen-fuzz-y");
- if (maximum || fuzz)
+ if (maximum || fuzz) {
touchscreen_set_params(dev, ABS_Y, maximum, fuzz);
+ touchscreen_set_params(dev, ABS_MT_POSITION_Y, maximum, fuzz);
+ }
maximum = of_get_optional_u32(np, "touchscreen-max-pressure");
fuzz = of_get_optional_u32(np, "touchscreen-fuzz-pressure");
- if (maximum || fuzz)
+ if (maximum || fuzz) {
touchscreen_set_params(dev, ABS_PRESSURE, maximum, fuzz);
+ touchscreen_set_params(dev, ABS_MT_PRESSURE, maximum, fuzz);
+ }
}
EXPORT_SYMBOL(touchscreen_parse_of_params);
--
2.3.0
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 3/4] input: ft5x06: Allow to set the maximum axes value through the DT
Currently the driver relies on some obscure and undocumented register to set
the maximum axis value.
The reported value is way too high to be meaningful, which confuses some
userspace tools like QT's evdevtouch plugin which try to scale the reported
events to the maximum values.
Use the values from the DT to set meaningful values.
Signed-off-by: Maxime Ripard
---
drivers/input/touchscreen/edt-ft5x06.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/input/touchscreen/edt-ft5x06.c
b/drivers/input/touchscreen/edt-ft5x06.c
index d4c24fb7704f..cd8a7472266b 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -37,6 +37,7 @@
#include
#include
#include
+#include
#include
#define MAX_SUPPORT_POINTS 5
@@ -1044,6 +1045,10 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
0, tsdata->num_x * 64 - 1, 0, 0);
input_set_abs_params(input, ABS_MT_POSITION_Y,
0, tsdata->num_y * 64 - 1, 0, 0);
+
+ if (!pdata)
+ touchscreen_parse_of_params(input);
+
error = input_mt_init_slots(input, MAX_SUPPORT_POINTS, 0);
if (error) {
dev_err(&client->dev, "Unable to init MT slots.\n");
--
2.3.0
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 0/4] input: ft5x06: Fix userspace reported maximum value
Hi, The current ft5x06 reports to the user-space that its maximum coordinates are, on both X and Y, way higher than what could be actually usable on the screen (in my case, 5759x1151 instead of 480x800). This causes trouble on some userspace stacks that then try to re-scale these coordinates back to the framebuffer resolution, like QT does. Use the of_touchscreen code to find the real touchscreen limits in the DT case, and report that to the userspace. Thanks, Maxime Changes from v2: - Rebased on top of 4.0-rc1 Changes from v1: - Do not use input_set_abs_params anymore to avoid enabling an axis that was not enabled by the driver itself in of_touchscreen - Emit a warning when an axis is set in the DT but not enabled in the driver - Remove EV_SYN from the edt-ft5x06 driver Maxime Ripard (4): input: touchscreen: of: Use input_set_abs_params input: touchscreen: of: Register multitouch axes input: ft5x06: Allow to set the maximum axes value through the DT input: edt-ft5x06: Remove EV_SYN event report drivers/input/touchscreen/edt-ft5x06.c | 6 ++- drivers/input/touchscreen/of_touchscreen.c | 62 +- 2 files changed, 57 insertions(+), 11 deletions(-) -- 2.3.0 -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/4] input: touchscreen: of: Use input_set_abs_params
Drivers are still required to call input_set_abs_params for their axes, as if
they only use the touchscreen_parse_of_params function, the axis bit in absbit
won't be set.
Switch to using input_set_abs_params to fully setup each and every available
axis so that drivers will be able to solely use this function.
Signed-off-by: Maxime Ripard
---
drivers/input/touchscreen/of_touchscreen.c | 50 --
1 file changed, 40 insertions(+), 10 deletions(-)
diff --git a/drivers/input/touchscreen/of_touchscreen.c
b/drivers/input/touchscreen/of_touchscreen.c
index f8f9b84230b1..fe80d8ba7efa 100644
--- a/drivers/input/touchscreen/of_touchscreen.c
+++ b/drivers/input/touchscreen/of_touchscreen.c
@@ -13,6 +13,33 @@
#include
#include
+static u32 of_get_optional_u32(struct device_node *np,
+ const char *property)
+{
+ u32 val = 0;
+
+ of_property_read_u32(np, property, &val);
+
+ return val;
+}
+
+static void touchscreen_set_params(struct input_dev *dev,
+ unsigned long axis,
+ int max, int fuzz)
+{
+ struct input_absinfo *absinfo;
+
+ if (!test_bit(axis, dev->absbit)) {
+ dev_warn(&dev->dev,
+"DT specifies parameters but the axis is not set
up\n");
+ return;
+ }
+
+ absinfo = &dev->absinfo[axis];
+ absinfo->maximum = max;
+ absinfo->fuzz = fuzz;
+}
+
/**
* touchscreen_parse_of_params - parse common touchscreen DT properties
* @dev: device that should be parsed
@@ -24,22 +51,25 @@
void touchscreen_parse_of_params(struct input_dev *dev)
{
struct device_node *np = dev->dev.parent->of_node;
- struct input_absinfo *absinfo;
+ u32 maximum, fuzz;
input_alloc_absinfo(dev);
if (!dev->absinfo)
return;
- absinfo = &dev->absinfo[ABS_X];
- of_property_read_u32(np, "touchscreen-size-x", &absinfo->maximum);
- of_property_read_u32(np, "touchscreen-fuzz-x", &absinfo->fuzz);
+ maximum = of_get_optional_u32(np, "touchscreen-size-x");
+ fuzz = of_get_optional_u32(np, "touchscreen-fuzz-x");
+ if (maximum || fuzz)
+ touchscreen_set_params(dev, ABS_X, maximum, fuzz);
- absinfo = &dev->absinfo[ABS_Y];
- of_property_read_u32(np, "touchscreen-size-y", &absinfo->maximum);
- of_property_read_u32(np, "touchscreen-fuzz-y", &absinfo->fuzz);
+ maximum = of_get_optional_u32(np, "touchscreen-size-y");
+ fuzz = of_get_optional_u32(np, "touchscreen-fuzz-y");
+ if (maximum || fuzz)
+ touchscreen_set_params(dev, ABS_Y, maximum, fuzz);
- absinfo = &dev->absinfo[ABS_PRESSURE];
- of_property_read_u32(np, "touchscreen-max-pressure", &absinfo->maximum);
- of_property_read_u32(np, "touchscreen-fuzz-pressure", &absinfo->fuzz);
+ maximum = of_get_optional_u32(np, "touchscreen-max-pressure");
+ fuzz = of_get_optional_u32(np, "touchscreen-fuzz-pressure");
+ if (maximum || fuzz)
+ touchscreen_set_params(dev, ABS_PRESSURE, maximum, fuzz);
}
EXPORT_SYMBOL(touchscreen_parse_of_params);
--
2.3.0
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/4] input: ft5x06: Fix userspace reported maximum value
Hi Markus, On Mon, Feb 23, 2015 at 10:49:36PM +0100, Markus Pargmann wrote: > Hi, > > On Thu, Nov 13, 2014 at 03:06:54PM +0100, Maxime Ripard wrote: > > Hi, > > > > The current ft5x06 reports to the user-space that its maximum > > coordinates are, on both X and Y, way higher than what could be > > actually usable on the screen (in my case, 5759x1151 instead of > > 480x800). > > > > This causes trouble on some userspace stacks that then try to re-scale > > these coordinates back to the framebuffer resolution, like QT does. > > > > Use the of_touchscreen code to find the real touchscreen limits in the > > DT case, and report that to the userspace. > > > > Thanks, > > Maxime > > any news on this? Unfortunately, no. I never got any review on this, and somehow forgot about it. I will rebase the patches on 4.0, and resend them. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [linux-sunxi] Re: [PATCH] arm: sunxi: input: RFC: Add sysfs voltage for sun4i-lradc driver
On Tue, Jan 27, 2015 at 11:52:34AM +0100, Hans de Goede wrote: > Hi, > > On 27-01-15 10:49, Priit Laes wrote: > > > >On Tue, 2015-01-27 at 10:18 +0100, Maxime Ripard wrote: > >>Hi, > >> > >>On Mon, Jan 26, 2015 at 06:58:32PM +0200, Priit Laes wrote: > >>>--- > >> > >>Like Hans was pointing out, commit log and signed-off-by please > >> > >>> .../ABI/testing/sysfs-driver-input-sun4i-lradc | 4 ++ > >>> drivers/input/keyboard/sun4i-lradc-keys.c | 49 > >>>+- > >>> 2 files changed, 43 insertions(+), 10 deletions(-) > >>> create mode 100644 Documentation/ABI/testing/sysfs-driver-input- > >>>sun4i-lradc > >>> > >>>diff --git a/Documentation/ABI/testing/sysfs-driver-input-sun4i- > >>>lradc b/Documentation/ABI/testing/sysfs-driver-input-sun4i-lradc > >>>new file mode 100644 > >>>index 000..e4e6448 > >>>--- /dev/null > >>>+++ b/Documentation/ABI/testing/sysfs-driver-input-sun4i-lradc > >>>@@ -0,0 +1,4 @@ > >>>+What: /sys/class/input/input(x)/device/voltage > >>>+Date: February 2015 > >>>+Contact: Priit Laes > >>>+Description: ADC output voltage in microvolts or 0 if device is > >>>not opened. > >> > >>Why is it returning 0 when "device is not opened" ? What does that > >>even mean? You can't read that file without opening it. > > > >It means that something has to open the /dev/input/inputX device which > >sets up the ADC before the voltage can be read from the sysfs file. > > > >[...] > > > > > >> > >>As I told you already, if you're going to expose this an ADC in the > >>end, the proper solution is to use the IIO framework, not adding a > >>custom sysfs file. > > > >My intention was to expose just a simple debug output, so one can > >press the buttons and read the voltages for devicetree keymap. > > > >If anyone can suggest a simpler approach than current sysfs based one, > >I would do it. > > The android driver always uses 0.2V / 200mV steps, so what I do is > simply create a mapping with 200mV mapped to KEY_VOLUMEUP, 400mV mapped > to KEY_VOLUMEDOWN, etc. following the hardcoded android driver mapping: > > https://github.com/linux-sunxi/linux-sunxi/blob/sunxi-3.4/drivers/input/keyboard/sun4i-keyboard.c#L136 > > Usually this will be correct in one go, after testing one can shuffle > key codes as needed (usually not needed) and/or remove unused entries. > > With that said I do think that a sysfs file to see the actual voltages, > or a kernel parameter to printk them on keypress interrupt would be useful. > > I guess the printk option would be better as it would show the actual > keypress value read, not some semi-random sample. That wouldn't require that much code actually. Either using dev_dbg, or debugfs like Dmitry was suggesting would be two nice solutions I guess. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [linux-sunxi] Re: [PATCH] arm: sunxi: input: RFC: Add sysfs voltage for sun4i-lradc driver
On Tue, Jan 27, 2015 at 11:49:49AM +0200, Priit Laes wrote: > > On Tue, 2015-01-27 at 10:18 +0100, Maxime Ripard wrote: > > Hi, > > > > On Mon, Jan 26, 2015 at 06:58:32PM +0200, Priit Laes wrote: > > > --- > > > > Like Hans was pointing out, commit log and signed-off-by please > > > > > .../ABI/testing/sysfs-driver-input-sun4i-lradc | 4 ++ > > > drivers/input/keyboard/sun4i-lradc-keys.c | 49 > > > +- > > > 2 files changed, 43 insertions(+), 10 deletions(-) > > > create mode 100644 Documentation/ABI/testing/sysfs-driver-input- > > > sun4i-lradc > > > > > > diff --git a/Documentation/ABI/testing/sysfs-driver-input-sun4i- > > > lradc b/Documentation/ABI/testing/sysfs-driver-input-sun4i-lradc > > > new file mode 100644 > > > index 000..e4e6448 > > > --- /dev/null > > > +++ b/Documentation/ABI/testing/sysfs-driver-input-sun4i-lradc > > > @@ -0,0 +1,4 @@ > > > +What: /sys/class/input/input(x)/device/voltage > > > +Date: February 2015 > > > +Contact: Priit Laes > > > +Description: ADC output voltage in microvolts or 0 if device is > > > not opened. > > > > Why is it returning 0 when "device is not opened" ? What does that > > even mean? You can't read that file without opening it. > > It means that something has to open the /dev/input/inputX device which > sets up the ADC before the voltage can be read from the sysfs file. It's a bug. > > As I told you already, if you're going to expose this an ADC in the > > end, the proper solution is to use the IIO framework, not adding a > > custom sysfs file. > > My intention was to expose just a simple debug output, so one can > press the buttons and read the voltages for devicetree keymap. > > If anyone can suggest a simpler approach than current sysfs based one, > I would do it. But full blown iio driver is currently out of scope. For this kind of ADCs, it's really not that hard, and provide more or less the same interface. > Also, Carlo's (ccaione) initially submitted (?) driver for lradc > utilized iio subsystem. And it was dropped because no-one-would-use-this-to-retrieve-the-voltage-ever :) Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH] arm: sunxi: input: RFC: Add sysfs voltage for sun4i-lradc driver
pp->name); > + dev_err(dev, "%s: Invalid 'linux,code' property\n", > pp->name); > return -EINVAL; > } > > @@ -257,14 +273,26 @@ static int sun4i_lradc_probe(struct platform_device > *pdev) > if (error) > return error; > > - error = input_register_device(lradc->input); > + error = device_create_file(dev, &dev_attr_voltage); As I told you already, if you're going to expose this an ADC in the end, the proper solution is to use the IIO framework, not adding a custom sysfs file. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v5 0/4] ARM:sunxi:ps2 Added support for A10/A20 ps2 controller
On Sun, Jan 25, 2015 at 07:10:05PM +0530, Vishnu Patekar wrote: > This adds PS2 controller support for Allwinner A10, A20 SOCs. > > I've tested PS2 keyboard on A20 Olimex-Lime2 board. > Hans had tested previous patch on A10 as well. Applied 3 and 4. Thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH 2/2] ARM: dts: sun6i: Add resistive touchscreen controller node to dtsi
On Sat, Jan 24, 2015 at 10:33:48PM +0800, Chen-Yu Tsai wrote: > Now that we support the sun6i variant of the touchscreen controller, > add the device node to the dtsi so we can use it. > > Signed-off-by: Chen-Yu Tsai Applied, thanks, Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v4 3/5] ARM: sunxi: dts: Add PS2 nodes to dtsi for A10 and A20
On Wed, Jan 21, 2015 at 04:40:28PM +0530, Vishnu Patekar wrote: > On Wed, Jan 21, 2015 at 1:32 AM, Maxime Ripard > wrote: > > Hi Vishnu, > > > > On Fri, Jan 16, 2015 at 07:33:39PM +0530, Vishnu Patekar wrote: > >> Signed-off-by: VishnuPatekar > >> Signed-off-by: Hans de Goede > > > > Why is there Hans Signed-off here? > Hans has modified the A10 dtsi and also tested on A10 board. > Shouldn't I add him as Signed-off? Yeah, you should :) Thanks, Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [linux-sunxi] [PATCH v4 5/5] ARM: sunxi: dts: Add PS2 nodes for A20 lime2 board
On Wed, Jan 21, 2015 at 04:34:45PM +0530, Vishnu Patekar wrote:
> Hello,
>
> On Wed, Jan 21, 2015 at 1:37 AM, Maxime Ripard
> wrote:
> >
> > On Tue, Jan 20, 2015 at 05:02:06PM +, Iain Paton wrote:
> > > On 16/01/15 14:03, Vishnu Patekar wrote:
> > > > Signed-off-by: VishnuPatekar
> > > > ---
> > > > arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts | 12
> > > > 1 file changed, 12 insertions(+)
> > > >
> > > > diff --git a/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
> > > > b/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
> > > > index ed364d5..3365f12 100644
> > > > --- a/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
> > > > +++ b/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
> > > > @@ -113,6 +113,18 @@
> > > > status = "okay";
> > > > };
> > > >
> > > > + ps20: ps2@01c2a000 {
> > > > + pinctrl-names = "default";
> > > > + pinctrl-0 = <&ps20_pins_a>;
> > > > + status = "okay";
> > > > + };
> > > > +
> > > > + ps21: ps2@01c2a400 {
> > > > + pinctrl-names = "default";
> > > > + pinctrl-0 = <&ps21_pins_a>;
> > > > + status = "okay";
> > > > + };
> > > > +
> > > > i2c0: i2c@01c2ac00 {
> > > > pinctrl-names = "default";
> > > > pinctrl-0 = <&i2c0_pins_a>;
> > > >
> > >
> > > As the Lime2 doesn't actually have any PS2 connectors on the board,
> > > I'd prefer that these are not enabled unconditionally. Doing so
> > > only makes it more difficult for people who want to use these pins
> > > for other functions.
> > >
> > > Device tree overlays seem to be close to being merged, perhaps we
> > > could leave enabling this to an overlay?
> >
> > I already had the exact same reasoning, and this was even removed at
> > some point. For some reason, it was added back, and I don't really
> > know why.
> This was removed in consideration that these pins conflict with HDMI,
> however, these is not conflict.
>
> It was kept there as an example. But, as there is no ps2 connector on
> board, I've no problem to remove these nodes from Lime2 dts file.
Yet, probing a driver for a device connected to floating lines sounds
like a pretty bad idea.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [linux-sunxi] [PATCH v4 5/5] ARM: sunxi: dts: Add PS2 nodes for A20 lime2 board
On Tue, Jan 20, 2015 at 05:02:06PM +, Iain Paton wrote:
> On 16/01/15 14:03, Vishnu Patekar wrote:
> > Signed-off-by: VishnuPatekar
> > ---
> > arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts | 12
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
> > b/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
> > index ed364d5..3365f12 100644
> > --- a/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
> > +++ b/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
> > @@ -113,6 +113,18 @@
> > status = "okay";
> > };
> >
> > + ps20: ps2@01c2a000 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&ps20_pins_a>;
> > + status = "okay";
> > + };
> > +
> > + ps21: ps2@01c2a400 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&ps21_pins_a>;
> > + status = "okay";
> > + };
> > +
> > i2c0: i2c@01c2ac00 {
> > pinctrl-names = "default";
> > pinctrl-0 = <&i2c0_pins_a>;
> >
>
> As the Lime2 doesn't actually have any PS2 connectors on the board,
> I'd prefer that these are not enabled unconditionally. Doing so
> only makes it more difficult for people who want to use these pins
> for other functions.
>
> Device tree overlays seem to be close to being merged, perhaps we
> could leave enabling this to an overlay?
I already had the exact same reasoning, and this was even removed at
some point. For some reason, it was added back, and I don't really
know why.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH v4 4/5] ARM: sunxi: dts: Add A10/A20 PS2 pin muxing options
On Fri, Jan 16, 2015 at 07:33:40PM +0530, Vishnu Patekar wrote:
> Signed-off-by: VishnuPatekar
> ---
> arch/arm/boot/dts/sun4i-a10.dtsi | 14 ++
> arch/arm/boot/dts/sun7i-a20.dtsi | 14 ++
> 2 files changed, 28 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi
> b/arch/arm/boot/dts/sun4i-a10.dtsi
> index 2c31242..8fade3e 100644
> --- a/arch/arm/boot/dts/sun4i-a10.dtsi
> +++ b/arch/arm/boot/dts/sun4i-a10.dtsi
> @@ -629,6 +629,20 @@
> allwinner,drive = <0>;
> allwinner,pull = <0>;
> };
> +
> + ps20_pins_a: ps20@0 {
> + allwinner,pins = "PI20", "PI21";
> + allwinner,function = "ps2";
> + allwinner,drive = <0>;
> + allwinner,pull = <0>;
> + };
> +
> + ps21_pins_a: ps21@0 {
> + allwinner,pins = "PH12", "PH13";
> + allwinner,function = "ps2";
> + allwinner,drive = <0>;
> + allwinner,pull = <0>;
> + };
> };
>
> timer@01c20c00 {
> diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi
> b/arch/arm/boot/dts/sun7i-a20.dtsi
> index f35c691..f9dd274 100644
> --- a/arch/arm/boot/dts/sun7i-a20.dtsi
> +++ b/arch/arm/boot/dts/sun7i-a20.dtsi
> @@ -866,6 +866,20 @@
> allwinner,drive = <0>;
> allwinner,pull = <0>;
> };
> +
> + ps20_pins_a: ps20@0 {
> + allwinner,pins = "PI20", "PI21";
> + allwinner,function = "ps2";
> + allwinner,drive = <0>;
> + allwinner,pull = <0>;
> + };
> +
> + ps21_pins_a: ps21@0 {
> + allwinner,pins = "PH12", "PH13";
> + allwinner,function = "ps2";
> + allwinner,drive = <0>;
> + allwinner,pull = <0>;
> + };
> };
Same thing here, we're using defines now for the values in ,drive and
,pull, please use them.
Thanks,
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH v4 3/5] ARM: sunxi: dts: Add PS2 nodes to dtsi for A10 and A20
Hi Vishnu,
On Fri, Jan 16, 2015 at 07:33:39PM +0530, Vishnu Patekar wrote:
> Signed-off-by: VishnuPatekar
> Signed-off-by: Hans de Goede
Why is there Hans Signed-off here?
> ---
> arch/arm/boot/dts/sun4i-a10.dtsi | 16
> arch/arm/boot/dts/sun7i-a20.dtsi | 16
> 2 files changed, 32 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi
> b/arch/arm/boot/dts/sun4i-a10.dtsi
> index 7b4099f..2c31242 100644
> --- a/arch/arm/boot/dts/sun4i-a10.dtsi
> +++ b/arch/arm/boot/dts/sun4i-a10.dtsi
> @@ -795,5 +795,21 @@
> #address-cells = <1>;
> #size-cells = <0>;
> };
> +
> + ps20: ps2@01c2a000 {
> + compatible = "allwinner,sun4i-a10-ps2";
> + reg = <0x01c2a000 0x400>;
> + interrupts = <62>;
> + clocks = <&apb1_gates 6>;
> + status = "disabled";
> + };
> +
> + ps21: ps2@01c2a400 {
> + compatible = "allwinner,sun4i-a10-ps2";
> + reg = <0x01c2a400 0x400>;
> + interrupts = <63>;
> + clocks = <&apb1_gates 7>;
> + status = "disabled";
> + };
> };
> };
> diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi
> b/arch/arm/boot/dts/sun7i-a20.dtsi
> index e21ce59..f35c691 100644
> --- a/arch/arm/boot/dts/sun7i-a20.dtsi
> +++ b/arch/arm/boot/dts/sun7i-a20.dtsi
> @@ -1093,5 +1093,21 @@
> #interrupt-cells = <3>;
> interrupts = <1 9 0xf04>;
> };
> +
> + ps20: ps2@01c2a000 {
> + compatible = "allwinner,sun4i-a10-ps2";
> + reg = <0x01c2a000 0x400>;
> + interrupts = <0 62 4>;
> + clocks = <&apb1_gates 6>;
> + status = "disabled";
> + };
> +
> + ps21: ps2@01c2a400 {
> + compatible = "allwinner,sun4i-a10-ps2";
> + reg = <0x01c2a400 0x400>;
> + interrupts = <0 63 4>;
We recently switched to using includes and defines for these hardcoded
values.
Could you switch to it as well?
Thanks,
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH v3 4/9] ARM: dts: sun7i: cubieboard2: add axp209 regulator nodes
On Tue, Jan 13, 2015 at 06:30:51PM +0800, Chen-Yu Tsai wrote: > >> You could probably chain the regulators in the DT via xxx-supply > >> if it helps. But beyond that, it is hard to do anything meaningful > >> at this point. Modeling them as fixed regulators beyond our control > >> is simpler. We also do not have an IPSOUT regulator for the AXP. > >> > >> > Eventually, I think we would be able to remove > >> > sunxi-common-regulators.dtsi, or at least, expose the proper regulator > >> > hierarchy. > >> > >> The USB and SATA regulators are just GPIO enabled switches (MOSFETs) > >> or current limiters. I think these will always exist because of the > >> reference designs. Or do you want to move them back into the board > >> dts files? FWIW, I like it the way it is now. Not that I don't like > >> it to be accurate. > > > > At least providing the right hierarchy at the board level would be > > great. Adding the -supply property to all our fixed regulators > > wouldn't take too much code, and would be enough to model properly the > > regulator trees. > > These are all unrelated to axp209. You'd end up with > > vin-supply = <®_vcc5v>; > > for the usb and sata power regulators. You could put these > in sunxi-common-regulators, and override them for boards > that actually have something controllable. Ok. Fine then. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v3 0/9] ARM: sunxi: Support cpufreq on sun[457]i
On Mon, Jan 12, 2015 at 10:10:20AM +0100, Maxime Ripard wrote: > On Mon, Jan 12, 2015 at 12:34:00PM +0800, Chen-Yu Tsai wrote: > > Hi, > > > > This is v3 of the cpufreq support series for sunxi. The series has been > > rebased onto the latest sunxi-next. I've dropped all the patches Maxime > > merged. This includes "ARM: sunxi: Register cpufreq-dt for sun[45678]i" > > which was merged but not published yet. > > > > Individual changes since v2 are listed within each patch. > > > > > > Original cover letter follows: > > > > This series adds support cpufreq support for sun[457]i using cpufreq-dt. > > This also supports passive cpu cooling (thermal throttling) using thermal > > zones with the temperature sensor in the SoC. > > > > The operating points for the supported platforms were taken from the > > linux-sunxi FEX files repository. The majority of boards use the same > > settings. Only with sun7i do we see slight variations, either disabling > > some frequencies, or bumping up the voltage a bit. In either case this > > can be done by limiting the constraints for the supply regulator, or > > overriding the OPP table in the board dts file. > > > > On sun7i, there is an additional operating point not found in the FEX > > files, 960 MHz @ 1.4V, which is the full speed setting in both u-boot-sunxi > > and mainline u-boot. > > > > The series has been tested on the 4 boards I have. With cpufreq active, > > the effects are visible as a decrease in SoC internal temperature. > > Stability for the operating points has been tested using: > > > > > > http://linux-sunxi.org/Hardware_Reliability_Tests#Reliability_of_cpufreq_voltage.2Ffrequency_settings > > > > More real world usage feedback is appreciated. Thermal throttling hasn't > > been tested much, due to not being able to generate enough load without > > the GPU for the SoC to heat up. Also on sun4i, the temperature sensor > > still hasn't been calibrated, so the readings are highly inaccurate. > > Applied 2, 3, 6 and 8. And I just applied 4, 5, 7 and 9. Thanks! -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v3 4/9] ARM: dts: sun7i: cubieboard2: add axp209 regulator nodes
Hi,
On Mon, Jan 12, 2015 at 05:38:12PM +0800, Chen-Yu Tsai wrote:
> Hi,
>
> On Mon, Jan 12, 2015 at 5:06 PM, Maxime Ripard
> wrote:
> > Hi Chen-Yu,
> >
> > On Mon, Jan 12, 2015 at 12:34:04PM +0800, Chen-Yu Tsai wrote:
> >> This patch adds the regulator nodes for the axp209 by including
> >> the axp209 dtsi. As the inputs of these regulators are from the
> >> axp209's PS output, which is basically just a mux over the 2
> >> inputs, it is considered to be unregulated. Thus we do not provide
> >> input supply properties for them.
> >>
> >> The regulator names and constraints are based on the board
> >> schematics and the SoC datasheet.
> >>
> >> DCDC2 is used as the cpu power supply. This patch also references
> >> it from the cpu node.
> >>
> >> Also get rid of axp209 properties already set in axp209.dtsi.
> >>
> >> Signed-off-by: Chen-Yu Tsai
> >> ---
> >>
> >> changes since v2
> >>
> >> none
> >>
> >> changes since v1:
> >>
> >> - Use preprocessor include for axp209.dtsi
> >> - Remove incorrectly squashed axp209.dtsi patch
> >>
> >> ---
> >> arch/arm/boot/dts/sun7i-a20-cubieboard2.dts | 35
> >> +
> >> 1 file changed, 31 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
> >> b/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
> >> index 18fc5db9c976..ec1fc2c8b3e3 100644
> >> --- a/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
> >> +++ b/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
> >> @@ -88,13 +88,9 @@
> >> status = "okay";
> >>
> >> axp209: pmic@34 {
> >> - compatible = "x-powers,axp209";
> >> reg = <0x34>;
> >> interrupt-parent = <&nmi_intc>;
> >> interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> >> -
> >> - interrupt-controller;
> >> - #interrupt-cells = <1>;
> >> };
> >> };
> >>
> >> @@ -145,3 +141,34 @@
> >> status = "okay";
> >> };
> >> };
> >> +
> >> +#include "axp209.dtsi"
> >> +
> >> +&cpu0 {
> >> + cpu-supply = <®_dcdc2>;
> >> +};
> >> +
> >> +®_dcdc2 {
> >> + regulator-always-on;
> >> + regulator-min-microvolt = <100>;
> >> + regulator-max-microvolt = <145>;
> >> + regulator-name = "vdd-cpu";
> >> +};
> >> +
> >> +®_dcdc3 {
> >> + regulator-always-on;
> >> + regulator-min-microvolt = <100>;
> >> + regulator-max-microvolt = <140>;
> >> + regulator-name = "vdd-int-dll";
> >> +};
> >> +
> >> +®_ldo1 {
> >> + regulator-name = "vdd-rtc";
> >> +};
> >> +
> >> +®_ldo2 {
> >> + regulator-always-on;
> >> + regulator-min-microvolt = <300>;
> >> + regulator-max-microvolt = <300>;
> >> + regulator-name = "avcc";
> >> +};
> >
> > How do reg_vcc3v3 and the other reg used in this DT (ahci, USB) fit
> > into that?
>
> The following applies to boards that use AXP209.
>
> reg_vcc3v3 or reg_vcc3v0 is an external buck regulator with it's
> enable pin tied to EXTEN on the AXP. This pin is controllable,
> but we do not have the driver for it. It is possible that multiple
> regulators are tied to this pin. I suggest not touching it without
> the correct schematics.
>
> The source for usb and ahci regulators, or reg_vcc5v if you will,
> is either the unregulated 5v from the power supply or the usb otg
> port. For the Cubietruck, there's an additional uncontrollable
> boost regulator that boosts the lipo battery's power up to 5v.
>
> On some of the Olimex boards that use higher input voltages, they
> use an uncontrollable buck regulator to step down the voltage to
> 5v. The Olinuxino-Micro also has a boost regulator for the battery,
> tied to EXTEN.
>
> The AXP209 simply does not have enough outputs for all the needed
> voltages.
Hmm, yes, ok. It makes
Re: [PATCH v3 0/9] ARM: sunxi: Support cpufreq on sun[457]i
On Mon, Jan 12, 2015 at 12:34:00PM +0800, Chen-Yu Tsai wrote: > Hi, > > This is v3 of the cpufreq support series for sunxi. The series has been > rebased onto the latest sunxi-next. I've dropped all the patches Maxime > merged. This includes "ARM: sunxi: Register cpufreq-dt for sun[45678]i" > which was merged but not published yet. > > Individual changes since v2 are listed within each patch. > > > Original cover letter follows: > > This series adds support cpufreq support for sun[457]i using cpufreq-dt. > This also supports passive cpu cooling (thermal throttling) using thermal > zones with the temperature sensor in the SoC. > > The operating points for the supported platforms were taken from the > linux-sunxi FEX files repository. The majority of boards use the same > settings. Only with sun7i do we see slight variations, either disabling > some frequencies, or bumping up the voltage a bit. In either case this > can be done by limiting the constraints for the supply regulator, or > overriding the OPP table in the board dts file. > > On sun7i, there is an additional operating point not found in the FEX > files, 960 MHz @ 1.4V, which is the full speed setting in both u-boot-sunxi > and mainline u-boot. > > The series has been tested on the 4 boards I have. With cpufreq active, > the effects are visible as a decrease in SoC internal temperature. > Stability for the operating points has been tested using: > > > http://linux-sunxi.org/Hardware_Reliability_Tests#Reliability_of_cpufreq_voltage.2Ffrequency_settings > > More real world usage feedback is appreciated. Thermal throttling hasn't > been tested much, due to not being able to generate enough load without > the GPU for the SoC to heat up. Also on sun4i, the temperature sensor > still hasn't been calibrated, so the readings are highly inaccurate. Applied 2, 3, 6 and 8. There's still some discussions about the boards regulators, that apply to all of them, even though I commented on one in particular. Thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v3 4/9] ARM: dts: sun7i: cubieboard2: add axp209 regulator nodes
Hi Chen-Yu,
On Mon, Jan 12, 2015 at 12:34:04PM +0800, Chen-Yu Tsai wrote:
> This patch adds the regulator nodes for the axp209 by including
> the axp209 dtsi. As the inputs of these regulators are from the
> axp209's PS output, which is basically just a mux over the 2
> inputs, it is considered to be unregulated. Thus we do not provide
> input supply properties for them.
>
> The regulator names and constraints are based on the board
> schematics and the SoC datasheet.
>
> DCDC2 is used as the cpu power supply. This patch also references
> it from the cpu node.
>
> Also get rid of axp209 properties already set in axp209.dtsi.
>
> Signed-off-by: Chen-Yu Tsai
> ---
>
> changes since v2
>
> none
>
> changes since v1:
>
> - Use preprocessor include for axp209.dtsi
> - Remove incorrectly squashed axp209.dtsi patch
>
> ---
> arch/arm/boot/dts/sun7i-a20-cubieboard2.dts | 35
> +
> 1 file changed, 31 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
> b/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
> index 18fc5db9c976..ec1fc2c8b3e3 100644
> --- a/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
> +++ b/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
> @@ -88,13 +88,9 @@
> status = "okay";
>
> axp209: pmic@34 {
> - compatible = "x-powers,axp209";
> reg = <0x34>;
> interrupt-parent = <&nmi_intc>;
> interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> -
> - interrupt-controller;
> - #interrupt-cells = <1>;
> };
> };
>
> @@ -145,3 +141,34 @@
> status = "okay";
> };
> };
> +
> +#include "axp209.dtsi"
> +
> +&cpu0 {
> + cpu-supply = <®_dcdc2>;
> +};
> +
> +®_dcdc2 {
> + regulator-always-on;
> + regulator-min-microvolt = <100>;
> + regulator-max-microvolt = <145>;
> + regulator-name = "vdd-cpu";
> +};
> +
> +®_dcdc3 {
> + regulator-always-on;
> + regulator-min-microvolt = <100>;
> + regulator-max-microvolt = <140>;
> + regulator-name = "vdd-int-dll";
> +};
> +
> +®_ldo1 {
> + regulator-name = "vdd-rtc";
> +};
> +
> +®_ldo2 {
> + regulator-always-on;
> + regulator-min-microvolt = <300>;
> + regulator-max-microvolt = <300>;
> + regulator-name = "avcc";
> +};
How do reg_vcc3v3 and the other reg used in this DT (ahci, USB) fit
into that?
Eventually, I think we would be able to remove
sunxi-common-regulators.dtsi, or at least, expose the proper regulator
hierarchy.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH 01/17] ARM: sunxi: Register cpufreq-dt for sun[45678]i
On Tue, Jan 06, 2015 at 10:35:11AM +0800, Chen-Yu Tsai wrote: > On sun[45678]i, we have one cluster of identical cores sharing a > clock, which is ideal for using cpufreq-dt. Register a platform > device for cpufreq-dt. > > Signed-off-by: Chen-Yu Tsai Applied, thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v8 6/6] ARM: sunxi: Add AXP20x support multi_v7_defconfig
On Tue, Dec 23, 2014 at 10:53:14AM +0800, Chen-Yu Tsai wrote: > From: Carlo Caione > > Signed-off-by: Carlo Caione > Signed-off-by: Chen-Yu Tsai Applied, thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v8 5/6] ARM: sunxi: Add AXP20x support in defconfig
On Tue, Dec 23, 2014 at 10:53:13AM +0800, Chen-Yu Tsai wrote: > From: Carlo Caione > > Signed-off-by: Carlo Caione > Signed-off-by: Chen-Yu Tsai Applied, thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH 17/17] ARM: multi_v7_defconfig: Enable TOUCHSCREEN_SUN4I, CPU_THERMAL
On Tue, Jan 06, 2015 at 10:35:27AM +0800, Chen-Yu Tsai wrote: > This patch enables TOUCHSCREEN_SUN4I and CPU_THERMAL to enable cpufreq > support with passive cpu cooling (thermal throttling) on sunxi by default. > > Signed-off-by: Chen-Yu Tsai Applied, thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH 08/17] ARM: dts: sun7i: cubieboard2: add axp209 regulator nodes
Hi,
On Tue, Jan 06, 2015 at 10:35:18AM +0800, Chen-Yu Tsai wrote:
> This patch adds the regulator nodes for the axp209 by including
> the axp209 dtsi. As the inputs of these regulators are from the
> axp209's PS output, which is basically just a mux over the 2
> inputs, it is considered to be unregulated. Thus we do not provide
> input supply properties for them.
>
> The regulator names and constraints are based on the board
> schematics and the SoC datasheet.
>
> DCDC2 is used as the cpu power supply. This patch also references
> it from the cpu node.
>
> Also get rid of axp209 properties already set in axp209.dtsi.
>
> Signed-off-by: Chen-Yu Tsai
> ---
> arch/arm/boot/dts/axp209.dtsi | 4
> arch/arm/boot/dts/sun7i-a20-cubieboard2.dts | 35
> +
> 2 files changed, 35 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/axp209.dtsi b/arch/arm/boot/dts/axp209.dtsi
> index 3c38cbafe285..fec3364b2110 100644
> --- a/arch/arm/boot/dts/axp209.dtsi
> +++ b/arch/arm/boot/dts/axp209.dtsi
> @@ -54,6 +54,10 @@
> */
>
> &axp209 {
> + compatible = "x-powers,axp209";
> + interrupt-controller;
> + #interrupt-cells = <1>;
> +
This should be in your patch adding the AXP DTSI.
> regulators {
> /* Default work frequency for buck regulators */
> x-powers,dcdc-freq = <1500>;
> diff --git a/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
> b/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
> index 53680983461a..61e61c0eb829 100644
> --- a/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
> +++ b/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
> @@ -84,13 +84,9 @@
> status = "okay";
>
> axp209: pmic@34 {
> - compatible = "x-powers,axp209";
> reg = <0x34>;
> interrupt-parent = <&nmi_intc>;
> interrupts = <0 8>;
> -
> - interrupt-controller;
> - #interrupt-cells = <1>;
> };
> };
>
> @@ -141,3 +137,34 @@
> status = "okay";
> };
> };
> +
> +/include/ "axp209.dtsi"
We've switched to preprocessor includes, please use them instead.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH 16/17] ARM: sunxi_defconfig: Enable TOUCHSCREEN_SUN4I, CPUFREQ_DT, CPU_THERMAL
On Tue, Jan 06, 2015 at 10:35:26AM +0800, Chen-Yu Tsai wrote: > This patch enables TOUCHSCREEN_SUN4I, CPUFREQ_DT, and CPU_THERMAL to > enable cpufreq support with passive cpu cooling (thermal throttling) > by default. > > Signed-off-by: Chen-Yu Tsai Applied, thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH 06/17] ARM: dts: sun7i: Add cpu clock reference and operating points to dtsi
On Tue, Jan 06, 2015 at 10:35:16AM +0800, Chen-Yu Tsai wrote: > The cpu core is clocked from the "cpu" clock. Add a reference to it > in the first cpu node. Also add "cpu0" label to the node. > > The operating points were taken from the A20 FEX files in the > sunxi-boards repository. Not all boards have the same settings. The > settings in this patch are the most generic ones. > > Signed-off-by: Chen-Yu Tsai Applied, thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH 02/17] clk: sunxi: Propagate rate changes to parent for mux clocks
On Tue, Jan 06, 2015 at 10:35:12AM +0800, Chen-Yu Tsai wrote: > The cpu clock on sunxi machines is just a mux clock, which is normally > fed by the main PLL, but can be muxed to the main or low power oscillator. > > Make the mux clock propagate rate changes to its parent, so we can > change the clock rate of the PLL, and thus actually implement rate > changing on the cpu clock. > > This patch also removes the no reparenting limit. > > Signed-off-by: Chen-Yu Tsai Applied, thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH 04/17] ARM: dts: sunxi: Add dtsi for AXP209 PMIC
Hi,
On Tue, Jan 06, 2015 at 10:35:14AM +0800, Chen-Yu Tsai wrote:
> The AXP209 PMIC is used with some Allwinner SoCs. This patch adds
> a dtsi file listing all the regulator nodes. The regulators are
> initialized based on their device node names.
>
> Signed-off-by: Chen-Yu Tsai
> ---
> arch/arm/boot/dts/axp209.dtsi | 87
> +++
> 1 file changed, 87 insertions(+)
> create mode 100644 arch/arm/boot/dts/axp209.dtsi
>
> diff --git a/arch/arm/boot/dts/axp209.dtsi b/arch/arm/boot/dts/axp209.dtsi
> new file mode 100644
> index ..3c38cbafe285
> --- /dev/null
> +++ b/arch/arm/boot/dts/axp209.dtsi
> @@ -0,0 +1,87 @@
> +/*
> + * Copyright 2015 Chen-Yu Tsai
> + *
> + * Chen-Yu Tsai
> + *
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public
> + * License along with this file; if not, write to the Free
> + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
> + * MA 02110-1301 USA
> + *
> + * 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.
> + */
> +
> +/*
> + * AXP202/209 Integrated Power Management Chip
> + * http://www.x-powers.com/product/AXP20X.php
> + * http://dl.linux-sunxi.org/AXP/AXP209%20Datasheet%20v1.0_cn.pdf
> + */
> +
> +&axp209 {
> + regulators {
> + /* Default work frequency for buck regulators */
> + x-powers,dcdc-freq = <1500>;
> +
> + /* This section lists all the regulators */
I don't think this comment is necessary.
> + reg_dcdc2: dcdc2 {
> + };
> +
> + reg_dcdc3: dcdc3 {
> + };
> +
> + reg_ldo1: ldo1 {
> + /* LDO1 is a fixed output regulator */
> + regulator-always-on;
> + regulator-min-microvolt = <130>;
> + regulator-max-microvolt = <130>;
> + };
> +
> + reg_ldo2: ldo2 {
> + };
> +
> + reg_ldo3: ldo3 {
> + };
> +
> + reg_ldo4: ldo4 {
> + };
> +
> + reg_ldo5: ldo5 {
> + };
Some default names for the regulator would be great!
Thanks,
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH 01/17] ARM: sunxi: Register cpufreq-dt for sun[45678]i
Hi, On Tue, Jan 06, 2015 at 10:35:11AM +0800, Chen-Yu Tsai wrote: > On sun[45678]i, we have one cluster of identical cores sharing a > clock, which is ideal for using cpufreq-dt. Register a platform > device for cpufreq-dt. > > Signed-off-by: Chen-Yu Tsai While I don't have anything against this patch, I seem to recall that it was not necessary anymore, or that there was some work in the direction of removing that kind of code, have you looked into that? Thanks, Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH 05/17] ARM: dts: sunxi: Enable thermal sensor support for RTP on sun[457]i
On Tue, Jan 06, 2015 at 10:35:15AM +0800, Chen-Yu Tsai wrote: > Now that the resistive touchpanel driver supports thermal sensors, > add the "#thermal-sensor-cells" property as required by the thermal > framework. > > Signed-off-by: Chen-Yu Tsai Applied, thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCHv3 3/5] ARM: sunxi: dts: Add PS2 nodes to dtsi for A10 and A20
Hi Vishnu,
On Fri, Dec 12, 2014 at 11:55:46PM +0530, VishnuPatekar wrote:
> 1) Fixup the sun4i ps/2 nodes interrupt property, sun4i interrupts take
> only 1 specifier
>
> 2) dt bindings should use the compat string for the earliest version of the
> hardware which has the relevant hardware block, unless there are differences,
> the A10 and A20 ps2 controllers are identical, so for both sun4i-a10-ps2
> should be used as compat string, update the sun7i.dtsi ps2 entries to
> use the sun4i-a10-ps2 compat string.
This shouldn't be a changelog, but what this patch actually does.
> Signed-off-by: VishnuPatekar
> Signed-off-by: Hans de Goede
> ---
> arch/arm/boot/dts/sun4i-a10.dtsi | 17 +
> arch/arm/boot/dts/sun7i-a20.dtsi | 18 ++
> 2 files changed, 35 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi
> b/arch/arm/boot/dts/sun4i-a10.dtsi
> index 7b4099f..ef9a01c 100644
> --- a/arch/arm/boot/dts/sun4i-a10.dtsi
> +++ b/arch/arm/boot/dts/sun4i-a10.dtsi
> @@ -629,6 +629,7 @@
> allwinner,drive = <0>;
> allwinner,pull = <0>;
> };
> +
This is an uneeded change
> };
>
> timer@01c20c00 {
> @@ -795,5 +796,21 @@
> #address-cells = <1>;
> #size-cells = <0>;
> };
> +
> + ps20: ps2@01c2a000 {
> + compatible = "allwinner,sun4i-a10-ps2";
> + reg = <0x01c2a000 0x400>;
> + interrupts = <62>;
> + clocks = <&apb1_gates 6>;
> + status = "disabled";
> + };
> +
> + ps21: ps2@01c2a400 {
> + compatible = "allwinner,sun4i-a10-ps2";
> + reg = <0x01c2a400 0x400>;
> + interrupts = <63>;
> + clocks = <&apb1_gates 7>;
> + status = "disabled";
> + };
> };
> };
> diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi
> b/arch/arm/boot/dts/sun7i-a20.dtsi
> index e21ce59..6ab7714 100644
> --- a/arch/arm/boot/dts/sun7i-a20.dtsi
> +++ b/arch/arm/boot/dts/sun7i-a20.dtsi
> @@ -866,6 +866,7 @@
> allwinner,drive = <0>;
> allwinner,pull = <0>;
> };
> +
Ditto.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCHv2 3/3] ARM:dts:sunxi:ps2 dt nodes for A10/A20 PS2 controller.
Hi,
Please keep the Cc list.
On Wed, Dec 10, 2014 at 01:39:52AM +0530, Vishnu Patekar wrote:
> > > --- a/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
> > > +++ b/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
> > > @@ -112,6 +112,19 @@
> > > pinctrl-0 = <&uart0_pins_a>;
> > > status = "okay";
> > > };
> > > + /* PS2 0 and PS2 1 are disabled by default
> > > + To enable PS2 0 and PS2 1 uncomment below ps20 and ps21
> > nodes
> > > + Please note that ps20 pins conflict with HDMI on Lime2
> > Board*/
> > > + /*ps20: ps2@0x01c2a000 {
> > > + pinctrl-names = "default";
> > > + pinctrl-0 = <&ps2_0_pins>;
> > > + status = "okay";
> > > + };
> > > + ps21: ps2@0x01c2a400 {
> > > + pinctrl-names = "default";
> > > + pinctrl-0 = <&ps2_1_pins>;
> > > + status = "okay";
> > > + };*/
> >
> > Hmmm, no, no comments in the DTS.
> >
> > Especially when it's that trivial to enable.
>
> Okie, I'll just keep ps20 and ps21 nodes commented.
No, the comments themselves were great. But the nodes themselves
shouldn't be there in the first place.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCHv2 2/3] sunxi:drivers:input:ps2 Added sunxi A10/A20 ps2 driver
Hi, Remember to reply to all the recipients. On Tue, Dec 09, 2014 at 04:40:36PM +0530, Vishnu Patekar wrote: > > > Signed-off-by: vishnupatekar > > > --- > > > drivers/input/serio/Kconfig | 10 ++ > > > drivers/input/serio/Makefile|1 + > > > drivers/input/serio/sunxi-ps2.c | 364 > > +++ > > > 3 files changed, 375 insertions(+) > > > create mode 100644 drivers/input/serio/sunxi-ps2.c > > > > > > diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig > > > index bc2d474..3a7599c 100644 > > > --- a/drivers/input/serio/Kconfig > > > +++ b/drivers/input/serio/Kconfig > > > @@ -281,4 +281,14 @@ config HYPERV_KEYBOARD > > > To compile this driver as a module, choose M here: the module > > will > > > be called hyperv_keyboard. > > > > > > +config SERIO_SUNXI_PS2 > > > + tristate "Allwinner Sun4i-A10/Sun7i-A20 PS/2 controller" > > > > Allwinner A10 is enough > > > Okie, Should I change the config option as well, like SERIO_SUN4I_PS2 ? Yep. > > > + /*Set Clock Divider Register*/ > > > + clk_scdf = DIV_ROUND_UP(src_clk, PS2_SAMPLE_CLK) - 1; > > > + clk_pcdf = DIV_ROUND_UP(PS2_SAMPLE_CLK, PS2_SCLK) - 1; > > > > So this is actually a rounding down? > > > > Why not just using src_clk / PS2_SAMPLE_CLK directly? > > > > > + rval = (clk_scdf<<8) | clk_pcdf; > > > > Spaces between the operators. Remember, run checkpatch. > > > Okie. > > checkpatch could not catch this!! Well, then it should have :) > > > + writel(rval, drvdata->reg_base + PS2_REG_CLKDR); > > > + > > > + /*Set Global Control Register*/ > > > + rval = PS2_GCTL_RESET | PS2_GCTL_INTEN | PS2_GCTL_MASTER > > > + | PS2_GCTL_BUSEN; > > > + writel(rval, drvdata->reg_base + PS2_REG_GCTL); > > > > You seem to be reading/writing from the same registers than in your > > interrupt handler, don't you need some locking in here? > > > > Interrupt is getting enabled only after writing to GCTL register. > > do you think still we need to locking mechanism here? Not really, you don't know the state of the device before your driver is probed, and the interrupts are enabled as soon as request_irq. So you can very well have the device running with the interrupts running before this function is called. Plus you might get spurious interrupts. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCHv2 2/3] sunxi:drivers:input:ps2 Added sunxi A10/A20 ps2 driver
>name));
> + strlcpy(serio->phys, dev_name(dev), sizeof(serio->phys));
> +
> + /* Get IRQ for the device */
> + irq = platform_get_irq(pdev, 0);
> + if (!irq) {
> + dev_err(dev, "no IRQ found\n");
> + error = -ENXIO;
> + goto error_disable_clk;
> + }
> +
> + drvdata->irq = irq;
> + drvdata->serio = serio;
> + drvdata->dev = dev;
> + error = devm_request_threaded_irq(dev, drvdata->irq,
> + sunxips2_interrupt, NULL, 0, DRIVER_NAME, drvdata);
This really looks like a case for a regular request_irq.
> + if (error) {
> + dev_err(drvdata->dev, "Interrupt alloc failed %d:error:%d\n",
> + drvdata->irq, error);
> + goto error_disable_clk;
> + }
> +
> + serio_register_port(serio);
> + platform_set_drvdata(pdev, drvdata);
> +
> + return 0; /* success */
> +
> +error_disable_clk:
> + clk_disable_unprepare(drvdata->clk);
> +
> +err_free_mem:
> + kfree(serio);
> + return error;
> +}
> +
> +static int sunxips2_remove(struct platform_device *pdev)
> +{
> + struct sunxips2data *drvdata = platform_get_drvdata(pdev);
> +
> + serio_unregister_port(drvdata->serio);
> + disable_irq(drvdata->irq);
> +
> + if (!IS_ERR(drvdata->clk))
> + clk_disable_unprepare(drvdata->clk);
And you can drop that if.
> + kfree(drvdata->serio);
> +
> + return 0;
> +}
> +
> +/* Match table for of_platform binding */
> +static const struct of_device_id sunxips2_of_match[] = {
> + { .compatible = "allwinner,sun7i-a20-ps2", },
> + { .compatible = "allwinner,sun4i-a10-ps2", },
If the two really are compatible, you just need one of them, the A10
one in that case, since that's the earlier SoCs.
> + { },
> +};
> +
> +MODULE_DEVICE_TABLE(of, sunxips2_of_match);
> +
> +/*platform driver structure*/
> +static struct platform_driver sunxips2_of_driver = {
> + .probe = sunxips2_probe,
> + .remove = sunxips2_remove,
> + .driver = {
> + .name = DRIVER_NAME,
> + .of_match_table = sunxips2_of_match,
> + },
> +};
> +module_platform_driver(sunxips2_of_driver);
> +
> +MODULE_AUTHOR("Vishnu Patekar ");
> +MODULE_AUTHOR("Aaron.maoye ");
> +MODULE_DESCRIPTION("Sunxi PS/2 driver");
> +MODULE_LICENSE("GPL");
> --
> 1.7.9.5
>
It's looking great otherwise.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCHv2 3/3] ARM:dts:sunxi:ps2 dt nodes for A10/A20 PS2 controller.
Hi,
Your commit title should be less that 80 chars.
It should also be formatted using "ARM: sunxi: dts:" as a prefix (More
specific to less specific).
On Sun, Dec 07, 2014 at 04:45:19AM +0530, vishnupatekar wrote:
> Added ps2 nodes in lime2 board dts. By default ps20 and ps21 nodes are
> commented as ps20 pins conflict with HDMI connector
> on Lime2 Board.
>
> Signed-off-by: vishnupatekar
> ---
> arch/arm/boot/dts/sun4i-a10.dtsi| 27 +
> arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts | 13 ++
> arch/arm/boot/dts/sun7i-a20.dtsi| 29
> +++
> 3 files changed, 69 insertions(+)
This patch should also be split into three different patches:
- The one adding the nodes for the controller to the DTSI
- The one adding the pinctrl nodes
- The one enabling the controller on the board.
>
> diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi
> b/arch/arm/boot/dts/sun4i-a10.dtsi
> index 5e2ec2d..4726e8d 100644
> --- a/arch/arm/boot/dts/sun4i-a10.dtsi
> +++ b/arch/arm/boot/dts/sun4i-a10.dtsi
> @@ -615,6 +615,19 @@
> allwinner,drive = <0>;
> allwinner,pull = <0>;
> };
A newline here please.
> + ps2_0_pins: ps2_0@0 {
You called the node ps20, you probably want to call it the same way
here. And we usually have a suffix like pins_a whenever we have
several muxing options (as it's probably the case).
> + allwinner,pins = "PI20","PI21";
A whitespace after the comma please.
> + allwinner,function = "ps2";
> + allwinner,drive = <0>;
> + allwinner,pull = <0>;
> + };
Newline.
> + ps2_1_pins: ps2_1@0 {
> + allwinner,pins = "PH12","PH13";
> + allwinner,function = "ps2";
> + allwinner,drive = <0>;
> + allwinner,pull = <0>;
> + };
> +
> };
>
> timer@01c20c00 {
> @@ -781,5 +794,19 @@
> #address-cells = <1>;
> #size-cells = <0>;
> };
Newline.
> + ps20: ps2@0x01c2a000 {
Drop the 0x
> + compatible = "allwinner,sun4i-a10-ps2";
> + reg = <0x01c2a000 0x400>;
> + interrupts = <0 62 4>;
> + clocks = <&apb1_gates 6>;
> + status = "disabled";
> + };
Newline
> + ps21: ps2@0x01c2a400 {
> + compatible = "allwinner,sun4i-a10-ps2";
> + reg = <0x01c2a400 0x400>;
> + interrupts = <0 63 4>;
> + clocks = <&apb1_gates 7>;
> + status = "disabled";
> + };
> };
> };
> diff --git a/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
> b/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
> index ed364d5..5624e63 100644
> --- a/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
> +++ b/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
> @@ -112,6 +112,19 @@
> pinctrl-0 = <&uart0_pins_a>;
> status = "okay";
> };
> + /* PS2 0 and PS2 1 are disabled by default
> + To enable PS2 0 and PS2 1 uncomment below ps20 and ps21 nodes
> + Please note that ps20 pins conflict with HDMI on Lime2 Board*/
> + /*ps20: ps2@0x01c2a000 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&ps2_0_pins>;
> + status = "okay";
> + };
> + ps21: ps2@0x01c2a400 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&ps2_1_pins>;
> + status = "okay";
> + };*/
Hmmm, no, no comments in the DTS.
Especially when it's that trivial to enable.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCHv2 1/3] sunxi:dts-bindings:input:ps2 bindings for A10/A20 ps2.
Hi,
On Sun, Dec 07, 2014 at 04:45:17AM +0530, vishnupatekar wrote:
> A10 and A20 have same PS2 addresses, clocks, interrupts.
> added compatible as allwinner,sun4i-a10-ps2.
>
> Signed-off-by: vishnupatekar
You should have your real name here.
> ---
> .../bindings/input/allwinner,sunxi-ps2.txt | 23
>
> 1 file changed, 23 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/input/allwinner,sunxi-ps2.txt
>
> diff --git a/Documentation/devicetree/bindings/input/allwinner,sunxi-ps2.txt
> b/Documentation/devicetree/bindings/input/allwinner,sunxi-ps2.txt
> new file mode 100644
> index 000..3a8919a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/allwinner,sunxi-ps2.txt
> @@ -0,0 +1,23 @@
> +* Device tree bindings for Allwinner A10, A20 PS2 host controller
> +A20 PS2 is dual role controller(PS2 host and PS2 device). These
> +bindings are for PS2 host controller. IBM compliant IBM PS2 and
> AT-compatible keyboard and mouse can be connected.
This should be wrapped to 80 chars.
> +Required properties:
> +
> + - reg : Offset and length of the register set for the device.
> + - compatible : Should one of the following:
> + - "allwinner,sun7i-a20-ps2"
> + - "allwinner,sun4i-a10-ps2"
> + - interrupts : The interrupt line connected to the PS2.
> + - clocks : The gate clk connected to the PS2.
> +
> +
> +Example:
> + ps20: ps2@0x01c2a000 {
> + compatible = "allwinner,sun7i-a20-ps2";
> + reg = <0x01c2a000 0x400>;
> + interrupts = <0 62 4>;
> + clocks = <&apb1_gates 6>;
> + status = "disabled";
> + };
> --
> 1.7.9.5
>
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH 2/3] drivers:input:ps2 Added sunxi A20 ps2 driver, changed makefile and Kconfig
> +static void sunxips2_close(struct serio *pserio)
> +{
> + struct sunxips2data *drvdata = pserio->port_data;
> +
> + spin_lock(&drvdata->ps2_lock);
spin_lock_irqsave would be better I guess.
> + /* Disable the PS2 interrupts */
> + writel(0, drvdata->base_address + SW_PS2_GCTRL);
> + spin_unlock(&drvdata->ps2_lock);
> +}
> +
> +static int sunxips2_write(struct serio *pserio, unsigned char val)
> +{
> + struct sunxips2data *drvdata = (struct sunxips2data *)pserio->port_data;
> + u32 timeout = 1;
> +
> + do {
> + if (readl(drvdata->base_address + SW_PS2_FSTAT) &
> SWPS2_FSTA_TXRDY) {
> + writel(val, drvdata->base_address + SW_PS2_DATA);
> + return 0;
> + }
> + } while (timeout--);
Please use time_before() here
> + return -1;
And return a meaningful error.
> +static int sunxips2_probe(struct platform_device *ofdev)
The ofdev name is measleading. This is a platform_device structure, it
could be probed by other mechanisms than OF, so please use a different
name here. Usually pdev is used.
> +{
> + struct resource *res; /* IO mem resources */
> + struct sunxips2data *drvdata;
> + struct serio *serio;
> + struct device *dev = &ofdev->dev;
> + unsigned int irq;
> + int error;
> +
> + drvdata = devm_kzalloc(dev, sizeof(struct sunxips2data), GFP_KERNEL);
> + serio = devm_kzalloc(dev, sizeof(struct serio), GFP_KERNEL);
> + if (!drvdata || !serio)
> + error = -ENOMEM;
> +
> + /* Request clock */
> + drvdata->pclk = clk_get(dev, NULL);
You can use devm_clk_get.
> + if (IS_ERR(drvdata->pclk))
> + dev_dbg(dev, "couldn't get clock %li\n",
> + PTR_ERR(drvdata->pclk));
This is an error, you should treat it as such.
> + if (!IS_ERR(drvdata->pclk)) {
> + error = clk_prepare_enable(drvdata->pclk);
> + if (error < 0) {
> + dev_err(dev, "failed to enable clock %d\n", error);
> + return error;
> + }
> + }
> +
> + /* IO */
> + res = platform_get_resource(ofdev, IORESOURCE_MEM, 0);
> + drvdata->base_address = devm_ioremap_resource(dev, res);
> + if (IS_ERR(drvdata->base_address)) {
> + dev_err(dev, "failed to map registers\n");
> + error = PTR_ERR(drvdata->base_address);
> + }
ditto.
> + serio->id.type = SERIO_8042;
> + serio->write = sunxips2_write;
> + serio->open = sunxips2_open;
> + serio->close = sunxips2_close;
> + serio->port_data = drvdata;
> + serio->dev.parent = dev;
> + strlcpy(serio->name, dev_name(dev), sizeof(serio->name));
> + strlcpy(serio->phys, dev_name(dev), sizeof(serio->phys));
> +
> + platform_set_drvdata(ofdev, drvdata);
> + serio_register_port(serio);
> +
> + /* Get IRQ for the device */
> + irq = irq_of_parse_and_map(ofdev->dev.of_node, 0);
> + if (!irq) {
> + dev_err(dev, "no IRQ found\n");
> + return -ENODEV;
> + }
You can use platform_get_irq, for consistency with how you retrieve
resources.
You're also no unregistering the driver from the serio framework.
> + drvdata->irq = irq;
> + drvdata->serio = serio;
> + drvdata->dev = dev;
> + error = devm_request_any_context_irq(drvdata->dev, drvdata->irq,
> &sunxips2_interrupt, 0,
> + DRIVER_NAME, drvdata);
request_irq is enough here.
> + if (error) {
> + dev_err(drvdata->dev,
> + "Couldn't allocate interrupt %d : error: %d\n",
> drvdata->irq, error);
> + return error;
> + }
> + return 0; /* success */
> +}
> +
> +static int sunxips2_remove(struct platform_device *of_dev)
> +{
> + struct sunxips2data *drvdata = platform_get_drvdata(of_dev);
> +
> + if (!IS_ERR(drvdata->pclk)) {
> + clk_disable_unprepare(drvdata->pclk);
> + clk_put(drvdata->pclk);
> + }
> + serio_unregister_port(drvdata->serio);
> + mdelay(2);
Why a mdelay?
> + return 0;
> +}
> +
> +/* Match table for of_platform binding */
> +static const struct of_device_id sunxips2_of_match[] = {
> + { .compatible = "allwinner,sun7i-a20-ps2", },
> + { },
> +};
> +
> +MODULE_DEVICE_TABLE(of, sunxips2_of_match);
> +
> +/*platform driver structure*/
> +static struct platform_driver sunxips2_of_driver = {
> + .probe = sunxips2_probe,
> + .remove = sunxips2_remove,
> + .driver = {
> + .name = DRIVER_NAME,
> + .owner = THIS_MODULE,
You can drop the owner field, it's already set by
module_platform_driver.
> + .of_match_table = sunxips2_of_match,
> + },
> +};
> +module_platform_driver(sunxips2_of_driver);
> +
> +MODULE_AUTHOR("Aaron.maoye + "Vishnu Patekar ");
Usually, you define two MODULE_AUTHOR.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
[PATCH v2 1/4] input: touchscreen: of: Use input_set_abs_params
Drivers are still required to call input_set_abs_params for their axes, as if
they only use the touchscreen_parse_of_params function, the axis bit in absbit
won't be set.
Switch to using input_set_abs_params to fully setup each and every available
axis so that drivers will be able to solely use this function.
Signed-off-by: Maxime Ripard
---
drivers/input/touchscreen/of_touchscreen.c | 50 --
1 file changed, 40 insertions(+), 10 deletions(-)
diff --git a/drivers/input/touchscreen/of_touchscreen.c
b/drivers/input/touchscreen/of_touchscreen.c
index f8f9b84230b1..fe80d8ba7efa 100644
--- a/drivers/input/touchscreen/of_touchscreen.c
+++ b/drivers/input/touchscreen/of_touchscreen.c
@@ -13,6 +13,33 @@
#include
#include
+static u32 of_get_optional_u32(struct device_node *np,
+ const char *property)
+{
+ u32 val = 0;
+
+ of_property_read_u32(np, property, &val);
+
+ return val;
+}
+
+static void touchscreen_set_params(struct input_dev *dev,
+ unsigned long axis,
+ int max, int fuzz)
+{
+ struct input_absinfo *absinfo;
+
+ if (!test_bit(axis, dev->absbit)) {
+ dev_warn(&dev->dev,
+"DT specifies parameters but the axis is not set
up\n");
+ return;
+ }
+
+ absinfo = &dev->absinfo[axis];
+ absinfo->maximum = max;
+ absinfo->fuzz = fuzz;
+}
+
/**
* touchscreen_parse_of_params - parse common touchscreen DT properties
* @dev: device that should be parsed
@@ -24,22 +51,25 @@
void touchscreen_parse_of_params(struct input_dev *dev)
{
struct device_node *np = dev->dev.parent->of_node;
- struct input_absinfo *absinfo;
+ u32 maximum, fuzz;
input_alloc_absinfo(dev);
if (!dev->absinfo)
return;
- absinfo = &dev->absinfo[ABS_X];
- of_property_read_u32(np, "touchscreen-size-x", &absinfo->maximum);
- of_property_read_u32(np, "touchscreen-fuzz-x", &absinfo->fuzz);
+ maximum = of_get_optional_u32(np, "touchscreen-size-x");
+ fuzz = of_get_optional_u32(np, "touchscreen-fuzz-x");
+ if (maximum || fuzz)
+ touchscreen_set_params(dev, ABS_X, maximum, fuzz);
- absinfo = &dev->absinfo[ABS_Y];
- of_property_read_u32(np, "touchscreen-size-y", &absinfo->maximum);
- of_property_read_u32(np, "touchscreen-fuzz-y", &absinfo->fuzz);
+ maximum = of_get_optional_u32(np, "touchscreen-size-y");
+ fuzz = of_get_optional_u32(np, "touchscreen-fuzz-y");
+ if (maximum || fuzz)
+ touchscreen_set_params(dev, ABS_Y, maximum, fuzz);
- absinfo = &dev->absinfo[ABS_PRESSURE];
- of_property_read_u32(np, "touchscreen-max-pressure", &absinfo->maximum);
- of_property_read_u32(np, "touchscreen-fuzz-pressure", &absinfo->fuzz);
+ maximum = of_get_optional_u32(np, "touchscreen-max-pressure");
+ fuzz = of_get_optional_u32(np, "touchscreen-fuzz-pressure");
+ if (maximum || fuzz)
+ touchscreen_set_params(dev, ABS_PRESSURE, maximum, fuzz);
}
EXPORT_SYMBOL(touchscreen_parse_of_params);
--
2.1.1
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/4] input: ft5x06: Allow to set the maximum axes value through the DT
Currently the driver relies on some obscure and undocumented register to set
the maximum axis value.
The reported value is way too high to be meaningful, which confuses some
userspace tools like QT's evdevtouch plugin which try to scale the reported
events to the maximum values.
Use the values from the DT to set meaningful values.
Signed-off-by: Maxime Ripard
---
drivers/input/touchscreen/edt-ft5x06.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/input/touchscreen/edt-ft5x06.c
b/drivers/input/touchscreen/edt-ft5x06.c
index ee3434f1e949..3ebc1a7e46c5 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -37,6 +37,7 @@
#include
#include
#include
+#include
#include
#define MAX_SUPPORT_POINTS 5
@@ -1042,6 +1043,10 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
0, tsdata->num_x * 64 - 1, 0, 0);
input_set_abs_params(input, ABS_MT_POSITION_Y,
0, tsdata->num_y * 64 - 1, 0, 0);
+
+ if (!pdata)
+ touchscreen_parse_of_params(input);
+
error = input_mt_init_slots(input, MAX_SUPPORT_POINTS, 0);
if (error) {
dev_err(&client->dev, "Unable to init MT slots.\n");
--
2.1.1
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/4] input: ft5x06: Fix userspace reported maximum value
Hi, The current ft5x06 reports to the user-space that its maximum coordinates are, on both X and Y, way higher than what could be actually usable on the screen (in my case, 5759x1151 instead of 480x800). This causes trouble on some userspace stacks that then try to re-scale these coordinates back to the framebuffer resolution, like QT does. Use the of_touchscreen code to find the real touchscreen limits in the DT case, and report that to the userspace. Thanks, Maxime Changes from v1: - Do not use input_set_abs_params anymore to avoid enabling an axis that was not enabled by the driver itself in of_touchscreen - Emit a warning when an axis is set in the DT but not enabled in the driver - Remove EV_SYN from the edt-ft5x06 driver Maxime Ripard (4): input: touchscreen: of: Use input_set_abs_params input: touchscreen: of: Register multitouch axes input: ft5x06: Allow to set the maximum axes value through the DT input: edt-ft5x06: Remove EV_SYN event report drivers/input/touchscreen/edt-ft5x06.c | 6 ++- drivers/input/touchscreen/of_touchscreen.c | 62 +- 2 files changed, 57 insertions(+), 11 deletions(-) -- 2.1.1 -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/4] input: edt-ft5x06: Remove EV_SYN event report
input_register_device already sets the EV_SYN event since all devices can generate them. Remove the redundant affectation. Signed-off-by: Maxime Ripard --- drivers/input/touchscreen/edt-ft5x06.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c index 3ebc1a7e46c5..a87f0d50f45c 100644 --- a/drivers/input/touchscreen/edt-ft5x06.c +++ b/drivers/input/touchscreen/edt-ft5x06.c @@ -1033,7 +1033,6 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client, input->id.bustype = BUS_I2C; input->dev.parent = &client->dev; - __set_bit(EV_SYN, input->evbit); __set_bit(EV_KEY, input->evbit); __set_bit(EV_ABS, input->evbit); __set_bit(BTN_TOUCH, input->keybit); -- 2.1.1 -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/4] input: touchscreen: of: Register multitouch axes
So far, the DT parsing code was only setting up the regular input axes,
completely ignoring their multitouch counter parts.
Fill them with the same parameters than the regular axes.
Signed-off-by: Maxime Ripard
---
drivers/input/touchscreen/of_touchscreen.c | 22 +-
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/drivers/input/touchscreen/of_touchscreen.c
b/drivers/input/touchscreen/of_touchscreen.c
index fe80d8ba7efa..b82b5207c78b 100644
--- a/drivers/input/touchscreen/of_touchscreen.c
+++ b/drivers/input/touchscreen/of_touchscreen.c
@@ -11,6 +11,7 @@
#include
#include
+#include
#include
static u32 of_get_optional_u32(struct device_node *np,
@@ -30,8 +31,13 @@ static void touchscreen_set_params(struct input_dev *dev,
struct input_absinfo *absinfo;
if (!test_bit(axis, dev->absbit)) {
- dev_warn(&dev->dev,
-"DT specifies parameters but the axis is not set
up\n");
+ /*
+* Emit a warning only if the axis is not a multitouch
+* axis, which might not be set by the driver.
+*/
+ if (!input_is_mt_axis(axis))
+ dev_warn(&dev->dev,
+"DT specifies parameters but the axis is not
set up\n");
return;
}
@@ -59,17 +65,23 @@ void touchscreen_parse_of_params(struct input_dev *dev)
maximum = of_get_optional_u32(np, "touchscreen-size-x");
fuzz = of_get_optional_u32(np, "touchscreen-fuzz-x");
- if (maximum || fuzz)
+ if (maximum || fuzz) {
touchscreen_set_params(dev, ABS_X, maximum, fuzz);
+ touchscreen_set_params(dev, ABS_MT_POSITION_X, maximum, fuzz);
+ }
maximum = of_get_optional_u32(np, "touchscreen-size-y");
fuzz = of_get_optional_u32(np, "touchscreen-fuzz-y");
- if (maximum || fuzz)
+ if (maximum || fuzz) {
touchscreen_set_params(dev, ABS_Y, maximum, fuzz);
+ touchscreen_set_params(dev, ABS_MT_POSITION_Y, maximum, fuzz);
+ }
maximum = of_get_optional_u32(np, "touchscreen-max-pressure");
fuzz = of_get_optional_u32(np, "touchscreen-fuzz-pressure");
- if (maximum || fuzz)
+ if (maximum || fuzz) {
touchscreen_set_params(dev, ABS_PRESSURE, maximum, fuzz);
+ touchscreen_set_params(dev, ABS_MT_PRESSURE, maximum, fuzz);
+ }
}
EXPORT_SYMBOL(touchscreen_parse_of_params);
--
2.1.1
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] input: touchscreen: of: Register multitouch axes
Hi,
On Wed, Nov 05, 2014 at 09:56:35AM -0800, Dmitry Torokhov wrote:
> On Wed, Nov 05, 2014 at 04:07:49PM +0100, Maxime Ripard wrote:
> > So far, the DT parsing code was only setting up the regular input axes,
> > completely ignoring their multitouch counter parts.
> >
> > Fill them with the same parameters than the regular axes.
> >
> > Signed-off-by: Maxime Ripard
> > ---
> > drivers/input/touchscreen/of_touchscreen.c | 12 +---
> > 1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/input/touchscreen/of_touchscreen.c
> > b/drivers/input/touchscreen/of_touchscreen.c
> > index 74d6b0eb50ac..cf2a753edd96 100644
> > --- a/drivers/input/touchscreen/of_touchscreen.c
> > +++ b/drivers/input/touchscreen/of_touchscreen.c
> > @@ -42,17 +42,23 @@ void touchscreen_parse_of_params(struct input_dev *dev)
> >
> > maximum = of_get_optional_u32(np, "touchscreen-size-x");
> > fuzz = of_get_optional_u32(np, "touchscreen-fuzz-x");
> > - if (maximum || fuzz)
> > + if (maximum || fuzz) {
> > input_set_abs_params(dev, ABS_X, 0, maximum, fuzz, 0);
> > + input_set_abs_params(dev, ABS_MT_POSITION_X, 0, maximum, fuzz,
> > 0);
>
> Not all devices are multitouch so you shoudl not be setting multitouch
> bits unconditionally.
Hmmm, right.
> In I think we should rely on driver to set capability bits properly and
> then here test them and apply the readings. Probably also issue a
> warning if we see max/fuzz setting but neither ABS_/ABS_MT_
> capabilities.
What I was aiming at was to avoid DT parsing duplication for the
!multitouch and multitouch axis. I don't think there's a way to copy
the parameters.
The mt_init_slots might be a solution, but it does the copy the other
way around: from the multitouch to the !multitouch axis, and without
enabling it, which renders using both input_mt_init_slots and the
of_touchscreen code together impossible.
Is there a way to just enable an axis without calling
input_set_abs_params? Is __set_bit enough?
Thanks,
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
[PATCH 2/3] input: touchscreen: of: Register multitouch axes
So far, the DT parsing code was only setting up the regular input axes,
completely ignoring their multitouch counter parts.
Fill them with the same parameters than the regular axes.
Signed-off-by: Maxime Ripard
---
drivers/input/touchscreen/of_touchscreen.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/input/touchscreen/of_touchscreen.c
b/drivers/input/touchscreen/of_touchscreen.c
index 74d6b0eb50ac..cf2a753edd96 100644
--- a/drivers/input/touchscreen/of_touchscreen.c
+++ b/drivers/input/touchscreen/of_touchscreen.c
@@ -42,17 +42,23 @@ void touchscreen_parse_of_params(struct input_dev *dev)
maximum = of_get_optional_u32(np, "touchscreen-size-x");
fuzz = of_get_optional_u32(np, "touchscreen-fuzz-x");
- if (maximum || fuzz)
+ if (maximum || fuzz) {
input_set_abs_params(dev, ABS_X, 0, maximum, fuzz, 0);
+ input_set_abs_params(dev, ABS_MT_POSITION_X, 0, maximum, fuzz,
0);
+ }
maximum = of_get_optional_u32(np, "touchscreen-size-y");
fuzz = of_get_optional_u32(np, "touchscreen-fuzz-y");
- if (maximum || fuzz)
+ if (maximum || fuzz) {
input_set_abs_params(dev, ABS_Y, 0, maximum, fuzz, 0);
+ input_set_abs_params(dev, ABS_MT_POSITION_Y, 0, maximum, fuzz,
0);
+ }
maximum = of_get_optional_u32(np, "touchscreen-max-pressure");
fuzz = of_get_optional_u32(np, "touchscreen-fuzz-pressure");
- if (maximum || fuzz)
+ if (maximum || fuzz) {
input_set_abs_params(dev, ABS_PRESSURE, 0, maximum, fuzz, 0);
+ input_set_abs_params(dev, ABS_MT_PRESSURE, 0, maximum, fuzz, 0);
+ }
}
EXPORT_SYMBOL(touchscreen_parse_of_params);
--
2.1.1
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] input: touchscreen: of: Use input_set_abs_params
Drivers are still required to call input_set_abs_params for their axes, as if
they only use the touchscreen_parse_of_params function, the axis bit in absbit
won't be set.
Switch to using input_set_abs_params to fully setup each and every available
axis so that drivers will be able to solely use this function.
Signed-off-by: Maxime Ripard
---
drivers/input/touchscreen/of_touchscreen.c | 33 +-
1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/drivers/input/touchscreen/of_touchscreen.c
b/drivers/input/touchscreen/of_touchscreen.c
index f8f9b84230b1..74d6b0eb50ac 100644
--- a/drivers/input/touchscreen/of_touchscreen.c
+++ b/drivers/input/touchscreen/of_touchscreen.c
@@ -13,6 +13,16 @@
#include
#include
+static u32 of_get_optional_u32(struct device_node *np,
+ const char *property)
+{
+ u32 val = 0;
+
+ of_property_read_u32(np, property, &val);
+
+ return val;
+}
+
/**
* touchscreen_parse_of_params - parse common touchscreen DT properties
* @dev: device that should be parsed
@@ -24,22 +34,25 @@
void touchscreen_parse_of_params(struct input_dev *dev)
{
struct device_node *np = dev->dev.parent->of_node;
- struct input_absinfo *absinfo;
+ u32 maximum, fuzz;
input_alloc_absinfo(dev);
if (!dev->absinfo)
return;
- absinfo = &dev->absinfo[ABS_X];
- of_property_read_u32(np, "touchscreen-size-x", &absinfo->maximum);
- of_property_read_u32(np, "touchscreen-fuzz-x", &absinfo->fuzz);
+ maximum = of_get_optional_u32(np, "touchscreen-size-x");
+ fuzz = of_get_optional_u32(np, "touchscreen-fuzz-x");
+ if (maximum || fuzz)
+ input_set_abs_params(dev, ABS_X, 0, maximum, fuzz, 0);
- absinfo = &dev->absinfo[ABS_Y];
- of_property_read_u32(np, "touchscreen-size-y", &absinfo->maximum);
- of_property_read_u32(np, "touchscreen-fuzz-y", &absinfo->fuzz);
+ maximum = of_get_optional_u32(np, "touchscreen-size-y");
+ fuzz = of_get_optional_u32(np, "touchscreen-fuzz-y");
+ if (maximum || fuzz)
+ input_set_abs_params(dev, ABS_Y, 0, maximum, fuzz, 0);
- absinfo = &dev->absinfo[ABS_PRESSURE];
- of_property_read_u32(np, "touchscreen-max-pressure", &absinfo->maximum);
- of_property_read_u32(np, "touchscreen-fuzz-pressure", &absinfo->fuzz);
+ maximum = of_get_optional_u32(np, "touchscreen-max-pressure");
+ fuzz = of_get_optional_u32(np, "touchscreen-fuzz-pressure");
+ if (maximum || fuzz)
+ input_set_abs_params(dev, ABS_PRESSURE, 0, maximum, fuzz, 0);
}
EXPORT_SYMBOL(touchscreen_parse_of_params);
--
2.1.1
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] input: ft5x06: Fix userspace reported maximum value
Hi, The current ft5x06 reports to the user-space that its maximum coordinates are, on both X and Y, way higher than what could be actually usable on the screen (in my case, 5759x1151 instead of 480x800). This causes trouble on some userspace stacks that then try to re-scale these coordinates back to the framebuffer resolution, like QT does. Use the of_touchscreen code to find the real touchscreen limits in the DT case, and report that to the userspace. Thanks, Maxime Maxime Ripard (3): input: touchscreen: of: Use input_set_abs_params input: touchscreen: of: Register multitouch axes input: ft5x06: Allow to set the maximum axes value through the DT drivers/input/touchscreen/edt-ft5x06.c | 5 drivers/input/touchscreen/of_touchscreen.c | 39 ++ 2 files changed, 34 insertions(+), 10 deletions(-) -- 2.1.1 -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] input: ft5x06: Allow to set the maximum axes value through the DT
Currently the driver relies on some obscure and undocumented register to set
the maximum axis value.
The reported value is way too high to be meaningful, which confuses some
userspace tools like QT's evdevtouch plugin which try to scale the reported
events to the maximum values.
Use the values from the DT to set meaningful values.
Signed-off-by: Maxime Ripard
---
drivers/input/touchscreen/edt-ft5x06.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/input/touchscreen/edt-ft5x06.c
b/drivers/input/touchscreen/edt-ft5x06.c
index ee3434f1e949..3ebc1a7e46c5 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -37,6 +37,7 @@
#include
#include
#include
+#include
#include
#define MAX_SUPPORT_POINTS 5
@@ -1042,6 +1043,10 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
0, tsdata->num_x * 64 - 1, 0, 0);
input_set_abs_params(input, ABS_MT_POSITION_Y,
0, tsdata->num_y * 64 - 1, 0, 0);
+
+ if (!pdata)
+ touchscreen_parse_of_params(input);
+
error = input_mt_init_slots(input, MAX_SUPPORT_POINTS, 0);
if (error) {
dev_err(&client->dev, "Unable to init MT slots.\n");
--
2.1.1
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/4] input: Add new sun4i-lradc-keys driver
Hi Hans, On Wed, Oct 22, 2014 at 01:11:02PM +0200, Hans de Goede wrote: > Allwinnner sunxi SoCs have a low resolution adc (called lradc) which is > specifically designed to have various (tablet) keys (ie home, back, search, > etc). attached to it using a resistor network. This adds a driver for this. > > There are 2 channels, currently this driver only supports chan0 since there > are no boards known to use chan1. > > This has been tested on an olimex a10s-olinuxino-micro, a13-olinuxino, and > a20-olinuxino-micro. > > Signed-off-by: Hans de Goede Acked-by: Maxime Ripard Thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v2 4/4] ARM: dts: sun7i: Add lradc node
Hi Hans, On Tue, Oct 21, 2014 at 10:24:50AM +0200, Hans de Goede wrote: > Signed-off-by: Hans de Goede > --- > arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts | 48 > + > arch/arm/boot/dts/sun7i-a20.dtsi| 7 > 2 files changed, 55 insertions(+) > > diff --git a/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts > b/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts > index 9d669cdf..85e7194 100644 > --- a/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts > +++ b/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts > @@ -14,6 +14,7 @@ > /dts-v1/; > /include/ "sun7i-a20.dtsi" > /include/ "sunxi-common-regulators.dtsi" > +#include I'm just wondering... Weren't we supposed to switch all includes to the preprocessor syntax in such a case? Maybe to handle the case were the DTSIs would have to use some preprocessors macros that wouldn't be expanded in this case? -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v7 0/6] mfd: AXP20x: Add support for AXP202 and AXP209
On Wed, Oct 22, 2014 at 08:28:42AM +0200, Bruno Prémont wrote: > On Sun, 29 Jun 2014 20:23:51 +0200 Carlo Caione wrote: > > During the merging of v6 several patches were left out. This v7 comprises > > all the patches that are still pending. > > Any progress on this or reason why these are stuck? No one bothered resubmitting it. If you have some time to work on this, it would be great if you could pick up the work, if not, I'll do it. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v2 1/4] input: Add new sun4i-lradc-keys driver
adc->input->phys = "sun4i_lradc/input0";
> + lradc->input->open = sun4i_lradc_open;
> + lradc->input->close = sun4i_lradc_close;
> + lradc->input->id.bustype = BUS_HOST;
> + lradc->input->id.vendor = 0x0001;
> + lradc->input->id.product = 0x0001;
> + lradc->input->id.version = 0x0100;
> + lradc->input->evbit[0] = BIT(EV_SYN) | BIT(EV_KEY);
> + for (i = 0; i < lradc->chan0_map_count; i++)
> + set_bit(lradc->chan0_map[i].keycode, lradc->input->keybit);
> + input_set_drvdata(lradc->input, lradc);
> +
> + lradc->base = devm_ioremap_resource(dev,
> + platform_get_resource(pdev, IORESOURCE_MEM, 0));
> + if (IS_ERR(lradc->base))
> + return PTR_ERR(lradc->base);
> +
> + ret = devm_request_irq(dev, platform_get_irq(pdev, 0), sun4i_lradc_irq,
> +0, "sun4i-lradc-keys", lradc);
> + if (ret)
> + return ret;
> +
> + ret = input_register_device(lradc->input);
> + if (ret)
> + return ret;
> +
> + platform_set_drvdata(pdev, lradc);
> + return 0;
> +}
> +
> +static const struct of_device_id sun4i_lradc_of_match[] = {
> + { .compatible = "allwinner,sun4i-lradc-keys", },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, sun4i_lradc_of_match);
> +
> +static struct platform_driver sun4i_lradc_driver = {
> + .driver = {
> + .owner = THIS_MODULE,
You can drop the owner field, it's already filled by
module_platform_driver.
> + .name = "sun4i-lradc-keys",
> + .of_match_table = of_match_ptr(sun4i_lradc_of_match),
> + },
> + .probe = sun4i_lradc_probe,
> +};
> +
> +module_platform_driver(sun4i_lradc_driver);
> +
> +MODULE_DESCRIPTION("Allwinner sun4i low res adc attached tablet keys
> driver");
> +MODULE_AUTHOR("Hans de Goede ");
> +MODULE_LICENSE("GPL");
> --
> 2.1.0
>
Looking good otherwise, thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH v4 0/2] input: Add new sun4i-ts driver for Allwinner sunxi SoC's rtp controller
Hi Hans, On Sun, May 11, 2014 at 10:06:24AM +0200, Hans de Goede wrote: > Hi All, > > Since Maxime rightfully pointed out an error in my resend of v3, here is v4 > with that fixed. > > Note the second patch which adds hwmon functionality has been reviewed by > the hwon subsystem maintainer, and the sun?i*.dts changes to enable this > have already been upstreamed. For both patches, Acked-by: Maxime Ripard Thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v3 1/2] input: Add new sun4i-ts driver for Allwinner sunxi SoC's rtp controller
Hi Hans, Thanks a lot for reposting this. On Sat, May 10, 2014 at 12:47:05PM +0200, Hans de Goede wrote: > Note the sun4i-ts controller is capable of detecting a second touch, but when > a second touch is present then the accuracy becomes so bad the reported touch > location is not useable. > > The original android driver contains some complicated heuristics using the > aprox. distance between the 2 touches to see if the user is making a pinch > open / close movement, and then reports emulated multi-touch events around > the last touch coordinate (as the dual-touch coordinates are worthless). > > These kinds of heuristics are just asking for trouble (and don't belong > in the kernel). So this driver offers straight forward, reliable single > touch functionality only. > > Signed-off-by: Hans de Goede > --- > .../bindings/input/touchscreen/sun4i.txt | 15 ++ > drivers/input/touchscreen/Kconfig | 10 + > drivers/input/touchscreen/Makefile | 1 + > drivers/input/touchscreen/sun4i-ts.c | 262 > + > 4 files changed, 288 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/input/touchscreen/sun4i.txt > create mode 100644 drivers/input/touchscreen/sun4i-ts.c > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt > b/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt > new file mode 100644 > index 000..e45927e > --- /dev/null > +++ b/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt > @@ -0,0 +1,15 @@ > +sun4i resistive touchscreen controller > +-- > + > +Required properties: > + - compatible: "allwinner,sun4i-ts" In between, I changed the compatible of this driver to sun4i-a10-ts to match the pattern used everywhere else. You should probably update it in your driver as well :) Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [linux-sunxi] Re: [PATCH v5 8/8] ARM: sun7i/sun4i: dt: Add AXP209 support to various boards
On Thu, May 08, 2014 at 10:17:16AM +0200, Carlo Caione wrote: > On Thu, May 8, 2014 at 4:53 AM, Maxime Ripard > wrote: > > > > > What I mean is that no information is still better than wrong > > informations. If you don't have the schematics, then don't do anything > > on this boards. Hopefully, someone with more infos on this will know > > what to do. > > I partially agree here. Not specifying the voltage range for the > regulators but only keeping them always-on from a practical point of > view means that we do nothing with those regulators (see also > https://www.mail-archive.com/linux-input@vger.kernel.org/msg09532.html > ) But we don't even know if they are used on these boards or not in the first place.. > > Moreover, I have the feeling that you just copy pasted all the > > informations on this patch. The first example I'm seeing is the > > cubieboard, for which we do have the schematics, and this patch says > > that ldo4 is used for HDMI, and that is required to stays always on, > > while LDO4 doesn't seem to be used at all, and that HDMI takes > > directly its input source from DC-5V. > > The names for the regulators are taken from the original Allwinner > driver and (of course) they can have nothing to do with the function > they have in the board. I agree that this could be misleading so I'll > change them with more general names. Concerning LDO4 that is always > on, it was off until this v4, but I changed it back to have one common > DT representation among all the boards. I'm sorry, you did a great job on this PMIC driver, but this is just wrong. I'm completely fine with the names. What I'm not fine with is all this "one common DT representation across all boards". Pretty much all the boards are wired differently when it comes to regulators. Why do you want to just do some copy pasting without any checking on the schematics side if not just for the joy of duplicating code? > > I guess you need some more refining on this patch. > > Ok, I'll submit a v6 only for this patch. Please, please, *please* make sure that the informations you put in there are actually accurate. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [linux-sunxi] Re: [PATCH v5 8/8] ARM: sun7i/sun4i: dt: Add AXP209 support to various boards
On Tue, May 06, 2014 at 09:38:23AM +0200, Carlo Caione wrote:
> On Tue, May 6, 2014 at 12:51 AM, Maxime Ripard
> wrote:
> > On Sat, May 03, 2014 at 02:21:06PM +0200, Carlo Caione wrote:
> >> On Sat, May 3, 2014 at 3:09 AM, Maxime Ripard
> >> wrote:
> >> > Hi,
> >> >
> >> > On Thu, May 01, 2014 at 02:29:34PM +0200, Carlo Caione wrote:
> >> >> Signed-off-by: Hans de Goede
> >> >> Signed-off-by: Carlo Caione
> >> >> ---
> >> >> arch/arm/boot/dts/sun4i-a10-a1000.dts | 58
> >> >> ++
> >> >> arch/arm/boot/dts/sun4i-a10-cubieboard.dts | 58
> >> >> ++
> >> >> arch/arm/boot/dts/sun4i-a10-hackberry.dts | 64
> >> >>
> >> >> arch/arm/boot/dts/sun4i-a10-inet97fv2.dts | 58
> >> >> ++
> >> >> arch/arm/boot/dts/sun4i-a10-mini-xplus.dts | 65
> >> >> +
> >> >> arch/arm/boot/dts/sun4i-a10-olinuxino-lime.dts | 64
> >> >>
> >> >> arch/arm/boot/dts/sun4i-a10-pcduino.dts | 58
> >> >> ++
> >> >> arch/arm/boot/dts/sun7i-a20-cubieboard2.dts | 59
> >> >> ++
> >> >> arch/arm/boot/dts/sun7i-a20-cubietruck.dts | 59
> >> >> ++
> >> >> arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts | 59
> >> >> ++
> >> >> 10 files changed, 602 insertions(+)
> >> >>
> >> >> diff --git a/arch/arm/boot/dts/sun4i-a10-a1000.dts
> >> >> b/arch/arm/boot/dts/sun4i-a10-a1000.dts
> >> >> index fa746aea..57d3fb4 100644
> >> >> --- a/arch/arm/boot/dts/sun4i-a10-a1000.dts
> >> >> +++ b/arch/arm/boot/dts/sun4i-a10-a1000.dts
> >> >> @@ -88,6 +88,56 @@
> >> >> pinctrl-names = "default";
> >> >> pinctrl-0 = <&i2c0_pins_a>;
> >> >> status = "okay";
> >> >> +
> >> >> + axp209: pmic@34 {
> >> >> + compatible = "x-powers,axp209";
> >> >> + reg = <0x34>;
> >> >> + interrupts = <0>;
> >> >> +
> >> >> + interrupt-controller;
> >> >> + #interrupt-cells = <1>;
> >> >> +
> >> >> + acin-supply = <®_axp_ipsout>;
> >> >> + vin2-supply = <®_axp_ipsout>;
> >> >> + vin3-supply = <®_axp_ipsout>;
> >> >> + ldo24in-supply = <®_axp_ipsout>;
> >> >> + ldo3in-supply = <®_axp_ipsout>;
> >> >> + ldo5in-supply = <®_axp_ipsout>;
> >> >> +
> >> >> + regulators {
> >> >> + x-powers,dcdc-freq = <1500>;
> >> >> +
> >> >> + axp_vcore_reg: dcdc2 {
> >> >> + regulator-min-microvolt =
> >> >> <70>;
> >> >> + regulator-max-microvolt =
> >> >> <2275000>;
> >> >> + regulator-always-on;
> >> >> + };
> >> >> +
> >> >> + axp_ddr_reg: dcdc3 {
> >> >> + regulator-always-on;
> >> >> + };
> >> >> +
> >> >> + axp_rtc_reg: ldo1 {
> >> >> + regulator-always-on;
> >> >> + };
> >> >> +
> >> >> + axp_analog_reg: ldo2 {
> >> >> + regulator-always-on;
Re: [linux-sunxi] Re: [PATCH v5 8/8] ARM: sun7i/sun4i: dt: Add AXP209 support to various boards
On Sat, May 03, 2014 at 02:21:06PM +0200, Carlo Caione wrote:
> On Sat, May 3, 2014 at 3:09 AM, Maxime Ripard
> wrote:
> > Hi,
> >
> > On Thu, May 01, 2014 at 02:29:34PM +0200, Carlo Caione wrote:
> >> Signed-off-by: Hans de Goede
> >> Signed-off-by: Carlo Caione
> >> ---
> >> arch/arm/boot/dts/sun4i-a10-a1000.dts | 58
> >> ++
> >> arch/arm/boot/dts/sun4i-a10-cubieboard.dts | 58
> >> ++
> >> arch/arm/boot/dts/sun4i-a10-hackberry.dts | 64
> >>
> >> arch/arm/boot/dts/sun4i-a10-inet97fv2.dts | 58
> >> ++
> >> arch/arm/boot/dts/sun4i-a10-mini-xplus.dts | 65
> >> +
> >> arch/arm/boot/dts/sun4i-a10-olinuxino-lime.dts | 64
> >>
> >> arch/arm/boot/dts/sun4i-a10-pcduino.dts | 58
> >> ++
> >> arch/arm/boot/dts/sun7i-a20-cubieboard2.dts | 59
> >> ++
> >> arch/arm/boot/dts/sun7i-a20-cubietruck.dts | 59
> >> ++
> >> arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts | 59
> >> ++
> >> 10 files changed, 602 insertions(+)
> >>
> >> diff --git a/arch/arm/boot/dts/sun4i-a10-a1000.dts
> >> b/arch/arm/boot/dts/sun4i-a10-a1000.dts
> >> index fa746aea..57d3fb4 100644
> >> --- a/arch/arm/boot/dts/sun4i-a10-a1000.dts
> >> +++ b/arch/arm/boot/dts/sun4i-a10-a1000.dts
> >> @@ -88,6 +88,56 @@
> >> pinctrl-names = "default";
> >> pinctrl-0 = <&i2c0_pins_a>;
> >> status = "okay";
> >> +
> >> + axp209: pmic@34 {
> >> + compatible = "x-powers,axp209";
> >> + reg = <0x34>;
> >> + interrupts = <0>;
> >> +
> >> + interrupt-controller;
> >> + #interrupt-cells = <1>;
> >> +
> >> + acin-supply = <®_axp_ipsout>;
> >> + vin2-supply = <®_axp_ipsout>;
> >> + vin3-supply = <®_axp_ipsout>;
> >> + ldo24in-supply = <®_axp_ipsout>;
> >> + ldo3in-supply = <®_axp_ipsout>;
> >> + ldo5in-supply = <®_axp_ipsout>;
> >> +
> >> + regulators {
> >> + x-powers,dcdc-freq = <1500>;
> >> +
> >> + axp_vcore_reg: dcdc2 {
> >> + regulator-min-microvolt =
> >> <70>;
> >> + regulator-max-microvolt =
> >> <2275000>;
> >> + regulator-always-on;
> >> + };
> >> +
> >> + axp_ddr_reg: dcdc3 {
> >> + regulator-always-on;
> >> + };
> >> +
> >> + axp_rtc_reg: ldo1 {
> >> + regulator-always-on;
> >> + };
> >> +
> >> + axp_analog_reg: ldo2 {
> >> + regulator-always-on;
> >> + };
> >> +
> >> + axp_pll_reg: ldo3 {
> >> + regulator-always-on;
> >> + };
> >> +
> >> + axp_hdmi_reg: ldo4 {
> >> + regulator-always-on;
> >> + };
> >> +
> >> + axp_mic_reg: ldo5 {
> >> + regulator-always-on;
> >
> > Do all these regulators need to be always on? It makes sense for the
> > pll and vcore, but I don't get why the mic and hdmi regulators need
> > this.
>
> I did this way because I don't have the schematics for all the board
> so I thought it was safer to leave all the regulators enabled
Well, we have the schematics for most of these boards (at least all
the cubie and olinuxinos).
As far as the other boards are concerned, if we don't have that
information, I'd say leave them alone until someone has access to
these informations.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH v5 4/8] input: misc: Add driver for AXP20x Power Enable Key
On Thu, May 01, 2014 at 02:29:30PM +0200, Carlo Caione wrote: > This patch add support for the Power Enable Key found on MFD AXP202 and > AXP209. Besides the basic support for the button, the driver adds two > entries in sysfs to configure the time delay for power on/off. > > Signed-off-by: Carlo Caione Acked-by: Maxime Ripard -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v5 8/8] ARM: sun7i/sun4i: dt: Add AXP209 support to various boards
Hi,
On Thu, May 01, 2014 at 02:29:34PM +0200, Carlo Caione wrote:
> Signed-off-by: Hans de Goede
> Signed-off-by: Carlo Caione
> ---
> arch/arm/boot/dts/sun4i-a10-a1000.dts | 58 ++
> arch/arm/boot/dts/sun4i-a10-cubieboard.dts | 58 ++
> arch/arm/boot/dts/sun4i-a10-hackberry.dts | 64
> arch/arm/boot/dts/sun4i-a10-inet97fv2.dts | 58 ++
> arch/arm/boot/dts/sun4i-a10-mini-xplus.dts | 65
> +
> arch/arm/boot/dts/sun4i-a10-olinuxino-lime.dts | 64
> arch/arm/boot/dts/sun4i-a10-pcduino.dts | 58 ++
> arch/arm/boot/dts/sun7i-a20-cubieboard2.dts | 59 ++
> arch/arm/boot/dts/sun7i-a20-cubietruck.dts | 59 ++
> arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts | 59 ++
> 10 files changed, 602 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun4i-a10-a1000.dts
> b/arch/arm/boot/dts/sun4i-a10-a1000.dts
> index fa746aea..57d3fb4 100644
> --- a/arch/arm/boot/dts/sun4i-a10-a1000.dts
> +++ b/arch/arm/boot/dts/sun4i-a10-a1000.dts
> @@ -88,6 +88,56 @@
> pinctrl-names = "default";
> pinctrl-0 = <&i2c0_pins_a>;
> status = "okay";
> +
> + axp209: pmic@34 {
> + compatible = "x-powers,axp209";
> + reg = <0x34>;
> + interrupts = <0>;
> +
> + interrupt-controller;
> + #interrupt-cells = <1>;
> +
> + acin-supply = <®_axp_ipsout>;
> + vin2-supply = <®_axp_ipsout>;
> + vin3-supply = <®_axp_ipsout>;
> + ldo24in-supply = <®_axp_ipsout>;
> + ldo3in-supply = <®_axp_ipsout>;
> + ldo5in-supply = <®_axp_ipsout>;
> +
> + regulators {
> + x-powers,dcdc-freq = <1500>;
> +
> + axp_vcore_reg: dcdc2 {
> + regulator-min-microvolt =
> <70>;
> + regulator-max-microvolt =
> <2275000>;
> + regulator-always-on;
> + };
> +
> + axp_ddr_reg: dcdc3 {
> + regulator-always-on;
> + };
> +
> + axp_rtc_reg: ldo1 {
> + regulator-always-on;
> + };
> +
> + axp_analog_reg: ldo2 {
> + regulator-always-on;
> + };
> +
> + axp_pll_reg: ldo3 {
> + regulator-always-on;
> + };
> +
> + axp_hdmi_reg: ldo4 {
> + regulator-always-on;
> + };
> +
> + axp_mic_reg: ldo5 {
> + regulator-always-on;
Do all these regulators need to be always on? It makes sense for the
pll and vcore, but I don't get why the mic and hdmi regulators need
this.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH v5 3/8] mfd: AXP20x: Add bindings documentation
Hi, On Thu, May 01, 2014 at 02:29:29PM +0200, Carlo Caione wrote: > Bindings documentation for the AXP20x driver. In this file also > sub-nodes are documented. > > Signed-off-by: Carlo Caione Acked-by: Maxime Ripard Thanks for your efforts, Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v5 1/8] mfd: AXP20x: Add mfd driver for AXP20x PMIC
On Thu, May 01, 2014 at 02:29:27PM +0200, Carlo Caione wrote: > This patch introduces the preliminary support for PMICs X-Powers AXP202 > and AXP209. The AXP209 and AXP202 are the PMUs (Power Management Unit) > used by A10, A13 and A20 SoCs and developed by X-Powers, a sister company > of Allwinner. > > The core enables support for two subsystems: > - PEK (Power Enable Key) > - Regulators > > Signed-off-by: Carlo Caione > Acked-by: Lee Jones Acked-by: Maxime Ripard Thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v4 7/9] ARM: sun7i/sun4i: dt: Add AXP209 support to various boards
ulator-min-microvolt =
> <180>;
> + regulator-max-microvolt =
> <330>;
> + regulator-always-on;
> + };
> +
> + axp_pll_reg: ldo3 {
> + regulator-min-microvolt =
> <70>;
> + regulator-max-microvolt =
> <350>;
> + };
> +
> + axp_hdmi_reg: ldo4 {
> + regulator-min-microvolt =
> <125>;
> + regulator-max-microvolt =
> <330>;
> + };
> +
> + axp_mic_reg: ldo5 {
> + regulator-min-microvolt =
> <180>;
> + regulator-max-microvolt =
> <330>;
> + };
> + };
> + };
> };
>
> i2c1: i2c@01c2b000 {
> diff --git a/arch/arm/boot/dts/sun4i-a10-hackberry.dts
> b/arch/arm/boot/dts/sun4i-a10-hackberry.dts
> index d7c17e4..8f2db9c 100644
> --- a/arch/arm/boot/dts/sun4i-a10-hackberry.dts
> +++ b/arch/arm/boot/dts/sun4i-a10-hackberry.dts
> @@ -82,6 +82,81 @@
> pinctrl-0 = <&uart0_pins_a>;
> status = "okay";
> };
> +
> + i2c0: i2c@01c2ac00 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c0_pins_a>;
> + status = "okay";
That should be in a separate patch.
These comments apply to most of the changes here. Make sure you edit
all of them.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH v3 08/10] ARM: sun7i/sun4i: dt: Add AXP209 support to various boards
On Fri, Mar 28, 2014 at 11:38:39AM +, Mark Brown wrote:
> On Fri, Mar 28, 2014 at 11:11:46AM +0100, Maxime Ripard wrote:
>
> > Here, you would just include the dtsi at the top of the file, and in
> > the DTSI, you would have something like:
>
> > &axp209 {
> >regulators {
> > ...
> >}
> > }
>
> Hang on, what is an include file setting up regulators for?
Look at patch 7. To share the regulators declaration across all boards
and avoid duplication.
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH v3 08/10] ARM: sun7i/sun4i: dt: Add AXP209 support to various boards
On Thu, Mar 27, 2014 at 10:29:22PM +0100, Carlo Caione wrote:
> Signed-off-by: Hans de Goede
> Signed-off-by: Carlo Caione
> ---
> arch/arm/boot/dts/sun4i-a10-a1000.dts | 13 +
> arch/arm/boot/dts/sun4i-a10-cubieboard.dts | 13 +
> arch/arm/boot/dts/sun4i-a10-hackberry.dts | 19 +++
> arch/arm/boot/dts/sun4i-a10-inet97fv2.dts | 13 +
> arch/arm/boot/dts/sun4i-a10-mini-xplus.dts | 19 +++
> arch/arm/boot/dts/sun4i-a10-olinuxino-lime.dts | 19 +++
> arch/arm/boot/dts/sun4i-a10-pcduino.dts | 13 +
> arch/arm/boot/dts/sun7i-a20-cubieboard2.dts | 14 ++
> arch/arm/boot/dts/sun7i-a20-cubietruck.dts | 15 +++
> arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts | 14 ++
> 10 files changed, 152 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun4i-a10-a1000.dts
> b/arch/arm/boot/dts/sun4i-a10-a1000.dts
> index fa746aea..cf18c4d 100644
> --- a/arch/arm/boot/dts/sun4i-a10-a1000.dts
> +++ b/arch/arm/boot/dts/sun4i-a10-a1000.dts
> @@ -88,6 +88,19 @@
> pinctrl-names = "default";
> pinctrl-0 = <&i2c0_pins_a>;
> status = "okay";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + axp: axp20x@34 {
> + reg = <0x34>;
> + interrupts = <0>;
> +
> + compatible = "x-powers,axp209";
> + interrupt-controller;
> + #interrupt-cells = <1>;
> +
> + /include/ "x-powers-axp209.dtsi"
> + };
> };
> };
Actually, you can make the whole thing much more robust by using the
&label syntax.
Here, you would just include the dtsi at the top of the file, and in
the DTSI, you would have something like:
&axp209 {
regulators {
...
}
}
(Given that the node in your DTS is axp209: pmic@something like
Chen-Yu suggested)
You don't rely any more on the position of the include, and you use a
standard DT syntax.
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [linux-sunxi] Re: [PATCH v2 6/8] ARM: sunxi: dt: Add x-powers-axp209.dtsi file
On Sat, Mar 22, 2014 at 03:31:57PM +0100, Carlo Caione wrote: > > > + compatible = "x-powers,axp209"; > > > + interrupt-controller; > > > + #interrupt-cells = <1>; > > > > However, I'd move this out of it, and in the board file, so that we > > actually get an idea by looking at the board DTS of what device we are > > actually registering at this given address, and what it's capable of. > > Do you mean the whole dtsi or just those three lines? Just those three lines. -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [linux-sunxi] Re: [PATCH v2 2/8] mfd: AXP20x: Add bindings documentation
On Sat, Mar 22, 2014 at 03:11:57PM +0100, Carlo Caione wrote:
> > > +
> > > +Example:
> > > +
> > > +axp: axp20x@34 {
> > > + reg = <0x34>;
> > > + interrupt-parent = <&nmi_intc>;
> > > + interrupts = <0 8>;
> > > +
> > > + compatible = "x-powers,axp209";
> > > + interrupt-controller;
> > > + #interrupt-cells = <1>;
> > > +
> > > + regulators {
> >
> > Do we really need that subnode ? it looks useless, and we already know
> > that we are defining regulators here.
>
> What do you mean? We are defining the MFD and regulators are just one of
> the subsystem here.
I mean that it's useless.
> Moveover I'm using the "regulators" node in the
> regulators driver (using of_find_node_by_name()) to get the regulators
> configuration.
Can't you just use the of_node field of whatever struct device you
get?
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH v2 3/8] input: misc: Add driver for AXP20x Power Enable Key
On Tue, Mar 18, 2014 at 10:58:51AM +, Lee Jones wrote:
> > > > > This patch add support for the Power Enable Key found on MFD AXP202
> > > > > and
> > > > > AXP209. Besides the basic support for the button, the driver adds two
> > > > > entries in sysfs to configure the time delay for power on/off.
> > > > >
> > > > > Signed-off-by: Carlo Caione
> > > > > ---
> > > > > drivers/input/misc/Kconfig | 11 ++
> > > > > drivers/input/misc/Makefile | 1 +
> > > > > drivers/input/misc/axp20x-pek.c | 267
> > > > >
> > > > > 3 files changed, 279 insertions(+)
> > > > > create mode 100644 drivers/input/misc/axp20x-pek.c
> > > >
> > > > From what I understood of the MFD framework, you usually have a MFD
> > > > core driver that gets loaded from the DT and instantiate its various
> > > > functions through sub-devices, that are registered through
> > > > mfd_add_devices, and the drivers for these sub-devices are supported
> > > > in sub-drivers that are located in the driver/mfd, alongside the core
> > > > driver.
> > > >
> > > > I believe that such a pattern allows for two interesting things:
> > > > - You don't have to search around the whole kernel tree to find
> > > > where a given sub-feature is supported.
> > > > - You don't have to cripple your DT with instantiation of all the
> > > > subcomponents, while you only really have one device.
> > > >
> > > > Do you have a reason for not following this pattern?
> > >
> > > Sorry Maxime, this is not the case.
> > >
> > > If an MFD contains Regulators and USB & GPIO Controllers, I'd expect
> > > to see the device represented in the following way:
> > >
> > > drivers/mfd/.c
> > > drivers/{gpio,pinctrl}/{gpio,pinctrl}-.c
> > > drivers/regulator/-regulator.c
> > > drivers/usb/host/.c
> >
> > Oh, ok. Nevermind then :)
> >
> > Just out of curiosity, some drivers at least seem to follow that trend
> > in drivers/mfd, is there any reason for this (other than historical) ?
>
> Would you mind providing an example?
I was under this impression given the naming scheme that they were
using. Looking into it just proved me how wrong I was :)
Sorry for the noise.
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH v2 3/8] input: misc: Add driver for AXP20x Power Enable Key
On Tue, Mar 18, 2014 at 09:50:02AM +, Lee Jones wrote:
> > > This patch add support for the Power Enable Key found on MFD AXP202 and
> > > AXP209. Besides the basic support for the button, the driver adds two
> > > entries in sysfs to configure the time delay for power on/off.
> > >
> > > Signed-off-by: Carlo Caione
> > > ---
> > > drivers/input/misc/Kconfig | 11 ++
> > > drivers/input/misc/Makefile | 1 +
> > > drivers/input/misc/axp20x-pek.c | 267
> > >
> > > 3 files changed, 279 insertions(+)
> > > create mode 100644 drivers/input/misc/axp20x-pek.c
> >
> > From what I understood of the MFD framework, you usually have a MFD
> > core driver that gets loaded from the DT and instantiate its various
> > functions through sub-devices, that are registered through
> > mfd_add_devices, and the drivers for these sub-devices are supported
> > in sub-drivers that are located in the driver/mfd, alongside the core
> > driver.
> >
> > I believe that such a pattern allows for two interesting things:
> > - You don't have to search around the whole kernel tree to find
> > where a given sub-feature is supported.
> > - You don't have to cripple your DT with instantiation of all the
> > subcomponents, while you only really have one device.
> >
> > Do you have a reason for not following this pattern?
>
> Sorry Maxime, this is not the case.
>
> If an MFD contains Regulators and USB & GPIO Controllers, I'd expect
> to see the device represented in the following way:
>
> drivers/mfd/.c
> drivers/{gpio,pinctrl}/{gpio,pinctrl}-.c
> drivers/regulator/-regulator.c
> drivers/usb/host/.c
Oh, ok. Nevermind then :)
Just out of curiosity, some drivers at least seem to follow that trend
in drivers/mfd, is there any reason for this (other than historical) ?
Thanks,
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH v2 8/8] ARM: sunxi: Add AXP20x support in defconfig
On Sat, Mar 15, 2014 at 04:43:45PM +0100, Carlo Caione wrote: > Signed-off-by: Carlo Caione > --- > arch/arm/configs/sunxi_defconfig | 4 > 1 file changed, 4 insertions(+) Could you add another patch doing the same with multi_v7 ? Thanks! -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v2 3/8] input: misc: Add driver for AXP20x Power Enable Key
dbf < 0) {
> + dev_err(&pdev->dev, "No IRQ for PEK_DBF, error=%d\n",
> + axp20x_pek->irq_dbf);
> + return axp20x_pek->irq_dbf;
> + }
> + axp20x_pek->irq_dbf = regmap_irq_get_virq(axp20x->regmap_irqc,
> + axp20x_pek->irq_dbf);
> +
> + axp20x_pek->input = devm_input_allocate_device(&pdev->dev);
> + if (!axp20x_pek->input)
> + return -ENOMEM;
> +
> + idev = axp20x_pek->input;
> +
> + idev->name = "axp20x-pek";
> + idev->phys = "m1kbd/input2";
> + idev->dev.parent = &pdev->dev;
> +
> + input_set_capability(idev, EV_KEY, KEY_POWER);
> +
> + input_set_drvdata(idev, axp20x_pek);
> +
> + error = devm_request_threaded_irq(&pdev->dev, axp20x_pek->irq_dbr,
> + NULL, axp20x_pek_irq, 0,
> + "axp20x-pek-dbr", idev);
> + if (error) {
> + dev_err(axp20x->dev, "Failed to request dbr IRQ#%d: %d\n",
> + axp20x_pek->irq_dbr, error);
> +
> + return error;
> + }
> +
> + error = devm_request_threaded_irq(&pdev->dev, axp20x_pek->irq_dbf,
> + NULL, axp20x_pek_irq, 0,
> + "axp20x-pek-dbf", idev);
> + if (error) {
> + dev_err(axp20x->dev, "Failed to request dbf IRQ#%d: %d\n",
> + axp20x_pek->irq_dbf, error);
> + return error;
> + }
> +
> + idev->dev.groups = dev_attr_groups;
> + error = input_register_device(idev);
> + if (error) {
> + dev_err(axp20x->dev, "Can't register input device: %d\n",
> error);
> + return error;
> + }
> +
> + platform_set_drvdata(pdev, axp20x_pek);
> +
> + return 0;
> +}
> +
> +static int axp20x_pek_remove(struct platform_device *pdev)
> +{
> + struct axp20x_pek *axp20x_pek = platform_get_drvdata(pdev);
> +
> + input_unregister_device(axp20x_pek->input);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id axp20x_pek_match[] = {
> + { .compatible = "x-powers,axp20x-pek", },
No pattern-matching compatible strings please.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH v2 6/8] ARM: sunxi: dt: Add x-powers-axp209.dtsi file
On Sat, Mar 15, 2014 at 04:43:43PM +0100, Carlo Caione wrote: > This dtsi describes the axp209 PMIC, and is to be included from inside > the i2c controller node to which the axp209 is connected. > > Signed-off-by: Hans de Goede > Signed-off-by: Carlo Caione > --- > arch/arm/boot/dts/x-powers-axp209.dtsi | 60 > ++ > 1 file changed, 60 insertions(+) > create mode 100644 arch/arm/boot/dts/x-powers-axp209.dtsi > > diff --git a/arch/arm/boot/dts/x-powers-axp209.dtsi > b/arch/arm/boot/dts/x-powers-axp209.dtsi > new file mode 100644 > index 000..d272e67 > --- /dev/null > +++ b/arch/arm/boot/dts/x-powers-axp209.dtsi > @@ -0,0 +1,60 @@ > +/* > + * x-powers,axp209 common code to be include from inside the axp209 node > + * > + * Copyright 2014 - Carlo Caione > + * > + * The code contained herein is licensed under the GNU General Public > + * License. You may obtain a copy of the GNU General Public License > + * Version 2 or later at the following locations: > + * > + * http://www.opensource.org/licenses/gpl-license.html > + * http://www.gnu.org/copyleft/gpl.html > + */ > + I didn't even know such a thing was possible :) Nice hack. > + compatible = "x-powers,axp209"; > + interrupt-controller; > + #interrupt-cells = <1>; However, I'd move this out of it, and in the board file, so that we actually get an idea by looking at the board DTS of what device we are actually registering at this given address, and what it's capable of. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v2 2/8] mfd: AXP20x: Add bindings documentation
On Sat, Mar 15, 2014 at 04:43:39PM +0100, Carlo Caione wrote:
> Bindings documentation for the AXP20x driver. In this file also two
> sub-nodes (PEK and regulators) are documented.
PEK doesn't look to be documented, either in this patch, or any other.
> Signed-off-by: Carlo Caione
> ---
> Documentation/devicetree/bindings/mfd/axp20x.txt | 83
> ++
> .../devicetree/bindings/vendor-prefixes.txt| 1 +
I don't really know what the DT maintainers are expecting here, but I
would have done two patches.
> 2 files changed, 84 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/axp20x.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/axp20x.txt
> b/Documentation/devicetree/bindings/mfd/axp20x.txt
> new file mode 100644
> index 000..982aefe
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/axp20x.txt
> @@ -0,0 +1,83 @@
> +* axp20x device tree bindings
> +
> +The axp20x family current members :-
> +axp202 (X-Powers)
> +axp209 (X-Powers)
> +
> +Required properties:
> +- compatible : Should be "x-powers,axp202" or
> "x-powers,axp209"
> +- interrupt-controller : axp20x has its own internal IRQs
> +- #interrupt-cells : Should be set to 1
> +- interrupt-parent : The parent interrupt controller
> +- interrupts : Interrupt specifiers for interrupt
> sources
> +- reg: The I2C slave address for the AXP chip
> +
> +Sub-nodes:
> +* regulators : Contain the regulator nodes. The regulators are bound using
> +their name as listed here: dcdc2, dcdc3, ldo1, ldo2, ldo3,
> +ldo4, ldo5.
> +The bindings details of individual regulator device can be found
> in:
> +Documentation/devicetree/bindings/regulator/regulator.txt with
> the
> +exception of:
> +
> + - dcdc-freq : defines the work frequency of DC-DC in KHz
> + (range: 750-1875). Default: 1.5MHz
> + - dcdc-workmode : Optional. 1 for PWM mode, 0 for AUTO mode
> + Default: AUTO mode
Those two are x-powers specific, or would it make sense to have them
in other drivers too?
If the former, please add the x-powers prefix.
> +
> +Example:
> +
> +axp: axp20x@34 {
> + reg = <0x34>;
> + interrupt-parent = <&nmi_intc>;
> + interrupts = <0 8>;
> +
> + compatible = "x-powers,axp209";
> + interrupt-controller;
> + #interrupt-cells = <1>;
> +
> + regulators {
Do we really need that subnode ? it looks useless, and we already know
that we are defining regulators here.
> + dcdc-freq = "1500";
That frequency is defined at the same level than the dcdc-workmode
property, yet they both seem to be placed at different levels.
> +
> + axp_dcdc2: dcdc2 {
> + regulator-min-microvolt = <70>;
> + regulator-max-microvolt = <2275000>;
> + dcdc-workmode = <0>;
> + regulator-always-on;
> + };
> +
> + axp_dcdc3: dcdc3 {
> + regulator-min-microvolt = <70>;
> + regulator-max-microvolt = <350>;
> + dcdc-workmode = <0>;
It looks like those are at their default values?
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH 1/7] mfd: AXP20x: Add mfd driver for AXP20x PMIC
Hi Carlo,
On Sat, Mar 08, 2014 at 12:31:41PM +0100, Carlo Caione wrote:
> > > + axp20x->pm_off = of_property_read_bool(node,
> > > "axp,system-power-controller");
> > > +
> > > + if (axp20x->pm_off && !pm_power_off) {
> > > + axp20x_pm_power_off = axp20x;
> > > + pm_power_off = axp20x_power_off;
> > > + }
> >
> > I don't think we have any other way to actually power down the board.
> >
> > Is there any reason why we would not need this property?
>
> In case you have multiple PMIC on the board in this way you con define
> which is the one apt to power off.
Do we even have such a board?
Let's leave this out, we can always add it later if needs be.
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH 6/7] regulator: AXP20x: Add support for regulators subsystem
On Sat, Mar 01, 2014 at 05:45:51PM +0100, Carlo Caione wrote:
> +static struct platform_driver axp20x_regulator_driver = {
> + .probe = axp20x_regulator_probe,
> + .remove = axp20x_regulator_remove,
> + .driver = {
> + .name = "axp20x-regulator",
> + .owner = THIS_MODULE,
> + },
> +};
> +
> +static int __init axp20x_regulator_init(void)
> +{
> + return platform_driver_register(&axp20x_regulator_driver);
> +}
> +
> +subsys_initcall(axp20x_regulator_init);
Hmmm, really?
I thought the AXP was only connected through I2C? How is that a
platform device then?
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH 4/7] input: misc: Add driver for AXP20x Power Enable Key
gt; + dev_err(&pdev->dev, "No IRQ for PEK_DBF, error=%d\n",
> + axp20x_pek->irq_dbf);
> + return axp20x_pek->irq_dbf;
> + }
> + axp20x_pek->irq_dbf = regmap_irq_get_virq(axp20x->regmap_irqc,
> + axp20x_pek->irq_dbf);
> +
> + axp20x_pek->input = devm_input_allocate_device(&pdev->dev);
> + if (!axp20x_pek->input)
> + return -ENOMEM;
> +
> + idev = axp20x_pek->input;
> +
> + idev->name = "axp20x-pek";
> + idev->phys = "m1kbd/input2";
> + idev->dev.parent = &pdev->dev;
> +
> + input_set_capability(idev, EV_KEY, KEY_POWER);
> +
> + input_set_drvdata(idev, axp20x_pek);
> +
> + error = devm_request_threaded_irq(&pdev->dev, axp20x_pek->irq_dbr,
> + NULL, axp20x_pek_irq, 0,
> + "axp20x-pek-dbr", idev);
> + if (error) {
> + dev_err(axp20x->dev, "Failed to request dbr IRQ#%d: %d\n",
> + axp20x_pek->irq_dbr, error);
> +
> + return error;
> + }
> +
> + error = devm_request_threaded_irq(&pdev->dev, axp20x_pek->irq_dbf,
> + NULL, axp20x_pek_irq, 0,
> + "axp20x-pek-dbf", idev);
> + if (error) {
> + dev_err(axp20x->dev, "Failed to request dbf IRQ#%d: %d\n",
> + axp20x_pek->irq_dbf, error);
> + return error;
> + }
> +
> + idev->dev.groups = dev_attr_groups;
> + error = input_register_device(idev);
> + if (error) {
> + dev_err(axp20x->dev, "Can't register input device: %d\n",
> error);
> + return error;
> + }
> +
> + platform_set_drvdata(pdev, axp20x_pek);
> +
> + return 0;
> +}
> +
> +static int axp20x_pek_remove(struct platform_device *pdev)
> +{
> + struct axp20x_pek *axp20x_pek = platform_get_drvdata(pdev);
> +
> + input_unregister_device(axp20x_pek->input);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id axp20x_pek_match[] = {
> + { .compatible = "x-powers,axp20x-pek", },
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, axp20x_pek_match);
> +
> +static struct platform_driver axp20x_pek_driver = {
> + .probe = axp20x_pek_probe,
> + .remove = axp20x_pek_remove,
> + .driver = {
> + .name = "axp20x-pek",
> + .owner = THIS_MODULE,
> + .of_match_table = axp20x_pek_match,
> + },
> +};
> +module_platform_driver(axp20x_pek_driver);
> +
> +MODULE_DESCRIPTION("axp20x Power Button");
> +MODULE_AUTHOR("Carlo Caione ");
> +MODULE_LICENSE("GPL");
> --
> 1.8.3.2
>
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH 2/7] mfd: AXP20x: Add bindings documentation
On Sat, Mar 01, 2014 at 05:45:47PM +0100, Carlo Caione wrote:
> Bindings documentation for the AXP20x driver. In this file also two
> sub-nodes (PEK and regulators) are documented.
>
> Signed-off-by: Carlo Caione
> ---
> Documentation/devicetree/bindings/mfd/axp20x.txt | 93
>
> 1 file changed, 93 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/axp20x.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/axp20x.txt
> b/Documentation/devicetree/bindings/mfd/axp20x.txt
> new file mode 100644
> index 000..ae3e3c4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/axp20x.txt
> @@ -0,0 +1,93 @@
> +* axp20x device tree bindings
> +
> +The axp20x family current members :-
> +axp202 (X-Powers)
> +axp209 (X-Powers)
> +
> +Required properties:
> +- compatible : Should be "x-powers,axp202" or
> "x-powers,axp209"
> +- interrupt-controller : axp20x has its own internal IRQs
> +- #interrupt-cells : Should be set to 1
> +- interrupt-parent : The parent interrupt controller
> +- interrupts : Interrupt specifiers for interrupt
> sources
> +- reg: The I2C slave address for the AXP chip
> +- axp,system-power-controller: Telling whether or not this pmic is
> + controlling the system power
> +
> +Sub-nodes:
> +* regulators : Contain the regulator nodes. The regulators are bound using
> +their name as listed here: dcdc2, dcdc3, ldo1, ldo2, ldo3,
> +ldo4, ldo5.
> +The bindings details of individual regulator device can be found
> in:
> +Documentation/devicetree/bindings/regulator/regulator.txt with
> the
> +exception of:
> +
> + - dcdc-freq : defines the work frequency of DC-DC in KHz
> + (range: 750-1875)
> + - dcdc-workmode : 1 for PWM mode, 0 for AUTO mode
You don't seem to always set this. You should mention that it is
optional, and which default value it has.
> +
> +* axp20x-pek : Power Enable Key
> + - compatible: should be "x-powers,axp20x-pek"
Why is this needed for?
Plus, please don't use any generic, or pattern matching compatibles,
but rather precise ones, so that if it is needed, we can add any quirk
we want.
> +Example:
> +
> +axp: axp20x@34 {
> + reg = <0x34>;
> + interrupt-parent = <&nmi_intc>;
> + interrupts = <0 8>;
> +
> + axp,system-power-controller;
> +
> + compatible = "x-powers,axp209";
> + interrupt-controller;
> + #interrupt-cells = <1>;
> +
> + axp20x-pek {
> + compatible = "x-powers,axp20x-pek";
> + };
> +
> + regulators {
> + dcdc-freq = "8";
> +
> + axp_dcdc2: dcdc2 {
> + regulator-min-microvolt = <70>;
> + regulator-max-microvolt = <2275000>;
> + dcdc-workmode = <0>;
> + regulator-always-on;
> + };
> +
> + axp_dcdc3: dcdc3 {
> + regulator-min-microvolt = <70>;
> + regulator-max-microvolt = <350>;
> + dcdc-workmode = <0>;
> + regulator-always-on;
> + };
> +
> + axp_ldo1: ldo1 {
> + regulator-min-microvolt = <130>;
> + regulator-max-microvolt = <130>;
> + };
> +
> + axp_ldo2: ldo2 {
> + regulator-min-microvolt = <180>;
> + regulator-max-microvolt = <330>;
> + regulator-always-on;
> + };
> +
> + axp_ldo3: ldo3 {
> + regulator-min-microvolt = <70>;
> + regulator-max-microvolt = <350>;
> + };
> +
> + axp_ldo4: ldo4 {
> + regulator-min-microvolt = <125>;
> + regulator-max-microvolt = <330>;
> + };
> +
> + axp_ldo5: ldo5 {
> + regulator-min-microvolt = <180>;
> + regulator-max-microvolt = <330>;
> + };
> + };
> +};
> +
> --
> 1.8.3.2
>
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH 1/7] mfd: AXP20x: Add mfd driver for AXP20x PMIC
0x40
> +#define AXP20X_IRQ2_EN 0x41
> +#define AXP20X_IRQ3_EN 0x42
> +#define AXP20X_IRQ4_EN 0x43
> +#define AXP20X_IRQ5_EN 0x44
> +#define AXP20X_IRQ1_STATE0x48
> +#define AXP20X_IRQ2_STATE0x49
> +#define AXP20X_IRQ3_STATE0x4a
> +#define AXP20X_IRQ4_STATE0x4b
> +#define AXP20X_IRQ5_STATE0x4c
> +
> +/* ADC */
> +#define AXP20X_ACIN_V_ADC_H 0x56
> +#define AXP20X_ACIN_V_ADC_L 0x57
> +#define AXP20X_ACIN_I_ADC_H 0x58
> +#define AXP20X_ACIN_I_ADC_L 0x59
> +#define AXP20X_VBUS_V_ADC_H 0x5a
> +#define AXP20X_VBUS_V_ADC_L 0x5b
> +#define AXP20X_VBUS_I_ADC_H 0x5c
> +#define AXP20X_VBUS_I_ADC_L 0x5d
> +#define AXP20X_TEMP_ADC_H0x5e
> +#define AXP20X_TEMP_ADC_L0x5f
> +#define AXP20X_TS_IN_H 0x62
> +#define AXP20X_TS_IN_L 0x63
> +#define AXP20X_GPIO0_V_ADC_H 0x64
> +#define AXP20X_GPIO0_V_ADC_L 0x65
> +#define AXP20X_GPIO1_V_ADC_H 0x66
> +#define AXP20X_GPIO1_V_ADC_L 0x67
> +#define AXP20X_PWR_BATT_H0x70
> +#define AXP20X_PWR_BATT_M0x71
> +#define AXP20X_PWR_BATT_L0x72
> +#define AXP20X_BATT_V_H 0x78
> +#define AXP20X_BATT_V_L 0x79
> +#define AXP20X_BATT_CHRG_I_H 0x7a
> +#define AXP20X_BATT_CHRG_I_L 0x7b
> +#define AXP20X_BATT_DISCHRG_I_H 0x7c
> +#define AXP20X_BATT_DISCHRG_I_L 0x7d
> +#define AXP20X_IPSOUT_V_HIGH_H 0x7e
> +#define AXP20X_IPSOUT_V_HIGH_L 0x7f
> +
> +/* Power supply */
> +#define AXP20X_DCDC_MODE 0x80
> +#define AXP20X_ADC_EN1 0x82
> +#define AXP20X_ADC_EN2 0x83
> +#define AXP20X_ADC_RATE 0x84
> +#define AXP20X_GPIO10_IN_RANGE 0x85
> +#define AXP20X_GPIO1_ADC_IRQ_RIS 0x86
> +#define AXP20X_GPIO1_ADC_IRQ_FAL 0x87
> +#define AXP20X_TIMER_CTRL0x8a
> +#define AXP20X_VBUS_MON 0x8b
> +#define AXP20X_OVER_TMP 0x8f
> +
> +/* GPIO */
> +#define AXP20X_GPIO0_CTRL0x90
> +#define AXP20X_LDO5_V_OUT0x91
> +#define AXP20X_GPIO1_CTRL0x92
> +#define AXP20X_GPIO2_CTRL0x93
> +#define AXP20X_GPIO20_SS 0x94
> +#define AXP20X_GPIO3_CTRL0x95
> +
> +/* Battery */
> +#define AXP20X_CHRG_CC_31_24 0xb0
> +#define AXP20X_CHRG_CC_23_16 0xb1
> +#define AXP20X_CHRG_CC_15_8 0xb2
> +#define AXP20X_CHRG_CC_7_0 0xb3
> +#define AXP20X_DISCHRG_CC_31_24 0xb4
> +#define AXP20X_DISCHRG_CC_23_16 0xb5
> +#define AXP20X_DISCHRG_CC_15_8 0xb6
> +#define AXP20X_DISCHRG_CC_7_00xb7
> +#define AXP20X_CC_CTRL 0xb8
> +#define AXP20X_FG_RES0xb9
> +
> +/* Regulators IDs */
> +enum {
> + AXP20X_LDO1 = 0,
> + AXP20X_LDO2,
> + AXP20X_LDO3,
> + AXP20X_LDO4,
> + AXP20X_LDO5,
> + AXP20X_DCDC2,
> + AXP20X_DCDC3,
> + AXP20X_REG_ID_MAX,
> +};
> +
> +/* IRQs */
> +enum {
> + AXP20X_IRQ_ACIN_OVER_V = 1,
> + AXP20X_IRQ_ACIN_PLUGIN,
> + AXP20X_IRQ_ACIN_REMOVAL,
> + AXP20X_IRQ_VBUS_OVER_V,
> + AXP20X_IRQ_VBUS_PLUGIN,
> + AXP20X_IRQ_VBUS_REMOVAL,
> + AXP20X_IRQ_VBUS_V_LOW,
> + AXP20X_IRQ_BATT_PLUGIN,
> + AXP20X_IRQ_BATT_REMOVAL,
> + AXP20X_IRQ_BATT_ENT_ACT_MODE,
> + AXP20X_IRQ_BATT_EXIT_ACT_MODE,
> + AXP20X_IRQ_CHARG,
> + AXP20X_IRQ_CHARG_DONE,
> + AXP20X_IRQ_BATT_TEMP_HIGH,
> + AXP20X_IRQ_BATT_TEMP_LOW,
> + AXP20X_IRQ_DIE_TEMP_HIGH,
> + AXP20X_IRQ_CHARG_I_LOW,
> + AXP20X_IRQ_DCDC1_V_LONG,
> + AXP20X_IRQ_DCDC2_V_LONG,
> + AXP20X_IRQ_DCDC3_V_LONG,
> + AXP20X_IRQ_PEK_SHORT = 22,
> + AXP20X_IRQ_PEK_LONG,
> + AXP20X_IRQ_N_OE_PWR_ON,
> + AXP20X_IRQ_N_OE_PWR_OFF,
> + AXP20X_IRQ_VBUS_VALID,
> + AXP20X_IRQ_VBUS_NOT_VALID,
> + AXP20X_IRQ_VBUS_SESS_VALID,
> + AXP20X_IRQ_VBUS_SESS_END,
> + AXP20X_IRQ_LOW_PWR_LVL1,
> + AXP20X_IRQ_LOW_PWR_LVL2,
> + AXP20X_IRQ_TIMER,
> + AXP20X_IRQ_PEK_RIS_EDGE,
> + AXP20X_IRQ_PEK_FAL_EDGE,
> + AXP20X_IRQ_GPIO3_INPUT,
> + AXP20X_IRQ_GPIO2_INPUT,
> + AXP20X_IRQ_GPIO1_INPUT,
> + AXP20X_IRQ_GPIO0_INPUT,
> +};
> +
> +struct axp20x_dev {
> + struct device *dev;
> + struct i2c_client *i2c_client;
> + struct regmap *regmap;
> + struct regmap_irq_chip_data *regmap_irqc;
> + int variant;
> + int irq;
> + boolpm_off;
> +};
> +
> +#endif /* __LINUX_MFD_AXP20X_H */
> --
> 1.8.3.2
>
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH 3/7] ARM: dts: cubieboard2: Add AXP209 support
On Sat, Mar 01, 2014 at 05:45:48PM +0100, Carlo Caione wrote:
> AXP209 is the PMU used by Cubieboard2. This patch enable the core
> support in the dts file.
> This patch requires: "ARM: sun7i/sun6i: irqchip: Add irqchip driver for
> NMI controller"
That shouldn't be in the commit log itself.
>
> Signed-off-by: Carlo Caione
> ---
> arch/arm/boot/dts/sun7i-a20-cubieboard2.dts | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
> b/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
> index 5c51cb8..234b14b 100644
> --- a/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
> +++ b/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
> @@ -53,6 +53,20 @@
> pinctrl-names = "default";
> pinctrl-0 = <&i2c0_pins_a>;
> status = "okay";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + axp: axp20x@34 {
> + reg = <0x34>;
> + interrupt-parent = <&nmi_intc>;
> + interrupts = <0 8>;
> +
> + axp,system-power-controller;
> +
> + compatible = "x-powers,axp209";
> + interrupt-controller;
> + #interrupt-cells = <1>;
> + };
> };
>
> i2c1: i2c@01c2b000 {
> --
> 1.8.3.2
>
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [lm-sensors] [PATCH v3 2/5] input: sun4i-ts: Add support for temperature sensor
Hi Guenter, On Sat, Jan 04, 2014 at 09:35:33AM -0800, Guenter Roeck wrote: > On 12/31/2013 08:20 AM, Hans de Goede wrote: > >The sun4i resisitive touchscreen controller also comes with a built-in > >temperature sensor. This commit adds support for it. > > > >This commit also introduces a new "ts-attached" device-tree property, > >when this is not set, the input part of the driver won't register. This way > >the internal temperature sensor can be used to measure the SoC temperature > >independent of there actually being a touchscreen attached to the controller. > > > >Signed-off-by: Hans de Goede > > Couple of minor comments below, though no need to resubmit unless someone > else has comments. > > Reviewed-by: Guenter Roeck > > >--- > > .../bindings/input/touchscreen/sun4i.txt | 5 + > > drivers/input/touchscreen/sun4i-ts.c | 140 > > - > > 2 files changed, 114 insertions(+), 31 deletions(-) > > > >diff --git a/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt > >b/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt > >index e45927e..6bac67b 100644 > >--- a/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt > >+++ b/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt > >@@ -6,10 +6,15 @@ Required properties: > > - reg: mmio address range of the chip > > - interrupts: interrupt to which the chip is connected > > > >+Optional properties: > >+ - allwinner,ts-attached: boolean indicating that an actual touchscreen is > >+ attached to the controller > >+ > Brr. While I understand that you were asked to do this, I don't > really see the benefit of another "allwinner" here. As if this > wasn't implied by the "compatible" property. It's actually the ePAPR that recommends this. Section 6.1.1, General Principles " Some recommended practices includes: [..] 5. If new properties are needed by the binding, the recommended format for property names is: “,”, where is an OUI or short unique string like a stock ticker that identifies the creator of the binding. " Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH 1/4] input: Add new sun4i-lradc-keys drivers
On Fri, Jan 03, 2014 at 10:23:50AM -0800, Dmitry Torokhov wrote:
> On Fri, Jan 03, 2014 at 06:36:05PM +0100, Maxime Ripard wrote:
> > On Thu, Jan 02, 2014 at 11:36:33PM +0100, Hans de Goede wrote:
> > > Hi,
> > >
> > > On 01/02/2014 09:20 PM, Maxime Ripard wrote:
> > > >On Thu, Jan 02, 2014 at 02:45:29PM +0100, Hans de Goede wrote:
> > > >>>Also, instead of inventing yet another vendor-specific property, why
> > > >>>not re-use
> > > >>>a button binding similar to gpio-keys like:
> > > >>>
> > > >>>lradc: lradc@01c22800 {
> > > >>>compatible = "allwinner,sun4i-lradc-keys";
> > > >>>reg = <0x01c22800 0x100>;
> > > >>>interrupts = <31>;
> > > >>>allwinner,chan0-step = <200>;
> > > >>>
> > > >>> #address-cells = <1>;
> > > >>> #size-cells = <0>;
> > > >>>
> > > >>> button@0 {
> > > >>> reg = <0>; /* your channel index from above */
> > > >>> linux,code = <115>; /* already used as
> > > >>> dt-property */
> > > >>> };
> > > >>>
> > > >>> button@1 {
> > > >>> reg = <1>;
> > > >>> linux,code = <114>;
> > > >>> };
> > > >>
> > > >>Ugh no. Having a vendor specific property which is KISS certainly
> > > >>beats this, both wrt ease of writing dts files as well as wrt the
> > > >>dts parsing code in the driver.
> > > >
> > > >I'd agree with Heiko here. This is pretty much the same construct
> > > >that's already in use in other input drivers, like gpio-keys.
> > >
> > > In the gpio case there is a 1 on 1 relation between a single hw
> > > entity (the gpio-pin) and a single keycode. Here there is 1 hw entity
> > > which maps to an array of key-codes, certainly using an array rather
> > > then a much more complicated construct is the correct data-structure
> > > to represent this.
> >
> > You can build an array in your driver out of this very easily, it's 10
> > lines in your probe. And you gain from this something that is more
> > generic, can be shared by other similar drivers and is consistent with
> > what is already in use.
>
> How will it be shared? Surely not code-wise, but basically in spirit
> only. It seems to me that the originally proposed binding is simple and
> concise and works well for the driver.
See Heiko's answer, but I do believe the code can be shared as well if
needs be.
> > > >This is also something that can really easily be made generic,
> > > >since this is something that is rather common.
> > > >
> > > >Speaking of which. I believe this should actually come in two
> > > >different drivers:
> > > > - The ADC driver itself, using IIO
> > > > - A generic button handler driver on top of IIO.
> > > >
> > > > The fact that on most board this adc is used for buttons doesn't make
> > > > any difference, it's actually a hardware designer choice, we should
> > > > support that choice, but we should also be able to use it just as an
> > > > ADC.
> > >
> > > No, this is not a generic adc, as mentioned in the commit msg, this
> > > adc is specifically designed to be used this way.
> > >
> > > The adc won't start sampling data, and won't generate any interrupts
> > > until a button is pressed. That is until the input voltage drops below
> > > 2/3 of Vref, this is checked through a built-in analog comparator, which
> > > hooks into the control logic.
> > >
> > > It has button down and button up interrupts, and can detect long
> > > presses (unused) and generate a second type of down interrupt for those.
> > >
> > > This really is an input device, which happens to use an adc.
> >
> > Hmm, yes, ok.
> >
> > > >Carlo Caione already started to work on an IIO driver for the LRADC:
> > > >https://github.com/carlocaione/linux/tree/sunxi-lradc
> > > >maybe you can take over his work.
> > >
>
Re: [PATCH 1/4] input: Add new sun4i-lradc-keys drivers
On Thu, Jan 02, 2014 at 11:36:33PM +0100, Hans de Goede wrote:
> Hi,
>
> On 01/02/2014 09:20 PM, Maxime Ripard wrote:
> >On Thu, Jan 02, 2014 at 02:45:29PM +0100, Hans de Goede wrote:
> >>>Also, instead of inventing yet another vendor-specific property, why not
> >>>re-use
> >>>a button binding similar to gpio-keys like:
> >>>
> >>>lradc: lradc@01c22800 {
> >>>compatible = "allwinner,sun4i-lradc-keys";
> >>>reg = <0x01c22800 0x100>;
> >>>interrupts = <31>;
> >>>allwinner,chan0-step = <200>;
> >>>
> >>> #address-cells = <1>;
> >>> #size-cells = <0>;
> >>>
> >>> button@0 {
> >>> reg = <0>; /* your channel index from above */
> >>> linux,code = <115>; /* already used as dt-property */
> >>> };
> >>>
> >>> button@1 {
> >>> reg = <1>;
> >>> linux,code = <114>;
> >>> };
> >>
> >>Ugh no. Having a vendor specific property which is KISS certainly
> >>beats this, both wrt ease of writing dts files as well as wrt the
> >>dts parsing code in the driver.
> >
> >I'd agree with Heiko here. This is pretty much the same construct
> >that's already in use in other input drivers, like gpio-keys.
>
> In the gpio case there is a 1 on 1 relation between a single hw
> entity (the gpio-pin) and a single keycode. Here there is 1 hw entity
> which maps to an array of key-codes, certainly using an array rather
> then a much more complicated construct is the correct data-structure
> to represent this.
You can build an array in your driver out of this very easily, it's 10
lines in your probe. And you gain from this something that is more
generic, can be shared by other similar drivers and is consistent with
what is already in use.
> >This is also something that can really easily be made generic,
> >since this is something that is rather common.
> >
> >Speaking of which. I believe this should actually come in two
> >different drivers:
> > - The ADC driver itself, using IIO
> > - A generic button handler driver on top of IIO.
> >
> > The fact that on most board this adc is used for buttons doesn't make
> > any difference, it's actually a hardware designer choice, we should
> > support that choice, but we should also be able to use it just as an
> > ADC.
>
> No, this is not a generic adc, as mentioned in the commit msg, this
> adc is specifically designed to be used this way.
>
> The adc won't start sampling data, and won't generate any interrupts
> until a button is pressed. That is until the input voltage drops below
> 2/3 of Vref, this is checked through a built-in analog comparator, which
> hooks into the control logic.
>
> It has button down and button up interrupts, and can detect long
> presses (unused) and generate a second type of down interrupt for those.
>
> This really is an input device, which happens to use an adc.
Hmm, yes, ok.
> >Carlo Caione already started to work on an IIO driver for the LRADC:
> >https://github.com/carlocaione/linux/tree/sunxi-lradc
> >maybe you can take over his work.
>
> That won't work because the adc won't sample if the input gets above
> 2/3 of Vref. There may be some other mode which does not do that, but
> that is not clearly documented.
>
> Even if an IIO driver turns out to be doable, I strongly believe that
> having a separate input driver for this is best, since this device
> was designed to be used as such. Building input on top of IIO would
> mean polling the adc, while with my driver it actually generates
> button down / up interrupts without any polling being involved.
Not really. iio_channel_read calls the read_raw function (in that
case) of your driver. If the read_raw function in your driver wants to
poll the device, fine, but most of the time, it will just block
waiting for an interrupt to come and return the data to the caller,
which is obviously the saner behaviour, and you don't actually end up
polling the device. Which is pretty much the architecture you're using
already, just with an intermediate layer in between.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH 1/4] input: Add new sun4i-lradc-keys drivers
On Thu, Jan 02, 2014 at 12:38:31PM -0800, Dmitry Torokhov wrote:
> On Thu, Jan 02, 2014 at 09:20:22PM +0100, Maxime Ripard wrote:
> > On Thu, Jan 02, 2014 at 02:45:29PM +0100, Hans de Goede wrote:
> > > >Also, instead of inventing yet another vendor-specific property, why not
> > > >re-use
> > > >a button binding similar to gpio-keys like:
> > > >
> > > >lradc: lradc@01c22800 {
> > > >compatible = "allwinner,sun4i-lradc-keys";
> > > >reg = <0x01c22800 0x100>;
> > > >interrupts = <31>;
> > > >allwinner,chan0-step = <200>;
> > > >
> > > > #address-cells = <1>;
> > > > #size-cells = <0>;
> > > >
> > > > button@0 {
> > > > reg = <0>; /* your channel index from above */
> > > > linux,code = <115>; /* already used as dt-property */
> > > > };
> > > >
> > > > button@1 {
> > > > reg = <1>;
> > > > linux,code = <114>;
> > > > };
> > >
> > > Ugh no. Having a vendor specific property which is KISS certainly
> > > beats this, both wrt ease of writing dts files as well as wrt the
> > > dts parsing code in the driver.
> >
> > I'd agree with Heiko here. This is pretty much the same construct
> > that's already in use in other input drivers, like gpio-keys.
> >
> > This is also something that can really easily be made generic, since
> > this is something that is rather common.
>
> Except that button definition from gpio-keys does not use 'reg' property
> but rather gpio. I'd rather we did not cram non-applicable attributes
> into that definition just to make it "reusable" like that.
>
> I'd be OK with having similar (but not claiming to be the same) mappings
> though.
Yes, this is what I was meaning. Sorry if it was not clear enough.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH 1/4] input: Add new sun4i-lradc-keys drivers
On Thu, Jan 02, 2014 at 02:45:29PM +0100, Hans de Goede wrote:
> >Also, instead of inventing yet another vendor-specific property, why not
> >re-use
> >a button binding similar to gpio-keys like:
> >
> >lradc: lradc@01c22800 {
> >compatible = "allwinner,sun4i-lradc-keys";
> >reg = <0x01c22800 0x100>;
> >interrupts = <31>;
> >allwinner,chan0-step = <200>;
> >
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > button@0 {
> > reg = <0>; /* your channel index from above */
> > linux,code = <115>; /* already used as dt-property */
> > };
> >
> > button@1 {
> > reg = <1>;
> > linux,code = <114>;
> > };
>
> Ugh no. Having a vendor specific property which is KISS certainly
> beats this, both wrt ease of writing dts files as well as wrt the
> dts parsing code in the driver.
I'd agree with Heiko here. This is pretty much the same construct
that's already in use in other input drivers, like gpio-keys.
This is also something that can really easily be made generic, since
this is something that is rather common.
Speaking of which. I believe this should actually come in two
different drivers:
- The ADC driver itself, using IIO
- A generic button handler driver on top of IIO.
The fact that on most board this adc is used for buttons doesn't make
any difference, it's actually a hardware designer choice, we should
support that choice, but we should also be able to use it just as an
ADC.
Carlo Caione already started to work on an IIO driver for the LRADC:
https://github.com/carlocaione/linux/tree/sunxi-lradc
maybe you can take over his work.
I also wonder wether it would be possible in that case to use reg as
the "button" voltage, to get rid of both the chan0-step property, and
those big fat arrays in the driver.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH v3 3/5] ARM: dts: sun4i: Add rtp controller node
On Tue, Dec 31, 2013 at 05:20:50PM +0100, Hans de Goede wrote: > Signed-off-by: Hans de Goede Applied, together with the patch 4/5 and 5/5, in my sunxi/dt-for-3.14 branch. Thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v2 2/5] input: sun4i-ts: Add support for temperature sensor
Hi, On Fri, Dec 27, 2013 at 05:13:04PM -0800, Dmitry Torokhov wrote: > On Fri, Dec 27, 2013 at 03:59:56PM +0100, Hans de Goede wrote: > > > > +Optional properties: > > + - ts-attached: boolean set this to tell the driver that an actual > > touchscreen > > + is attached and that it should register an input device, > > + without this it only registers the builtin temperate sensor > > I believe the guidance is to avoid describing kernel behavior in DT > property so I think it should say: > > - ts-attached: boolean indicating that an actual touchscreen is > attached to the controller. Custom properties are also supposed to be prefixed by the vendor prefix. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
