Re: [PATCH 3/3] input: dt-bindings: Add binding for bma150 sensor
On Sat, Jan 26, 2019 at 03:39:17AM +, Jonathan Bakker wrote:
>
>
> On 2019-01-25 5:28 p.m., Dmitry Torokhov wrote:
> > On Fri, Jan 25, 2019 at 07:44:00PM +0100, Paweł Chmiel wrote:
> >> From: Jonathan Bakker
> >>
> >> Add device tree bindings for Bosch BMA150 Accelerometer Sensor
> >>
> >> Signed-off-by: Jonathan Bakker
> >> Signed-off-by: Paweł Chmiel
> >> ---
> >> .../bindings/input/bosch,bma150.txt | 20 +++
> >> 1 file changed, 20 insertions(+)
> >> create mode 100644
> >> Documentation/devicetree/bindings/input/bosch,bma150.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/input/bosch,bma150.txt
> >> b/Documentation/devicetree/bindings/input/bosch,bma150.txt
> >> new file mode 100644
> >> index ..290c60e38c70
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/input/bosch,bma150.txt
> >> @@ -0,0 +1,20 @@
> >> +* Bosch BMA150 Accelerometer Sensor
> >> +
> >> +Also works for the SMB380 and BMA023 accelerometers
> >> +
> >> +Required properties:
> >> +- compatible : Should be "bosch,bma150"
> >> +- reg : The I2C address of the sensor
> >> +
> >> +Optional properties:
> >> +- interrupt-parent : should be the phandle for the interrupt controller
> >> +- interrupts : Interrupt mapping for IRQ. If not present device will be
> >> polled
> >> +
> >> +Example:
> >> +
> >> +bma150@38 {
> >> + compatible = "bosch,bma150";
> >> + reg = <0x38>;
> >> + interrupt-parent = <>;
> >> + interrupts = <1 IRQ_TYPE_LEVEL_HIGH>;
> >
> > Hmm, here you say that IRQ_TYPE_LEVEL_HIGH, so it is level interrupts,
> > but the driver overrides to rising edge unconditionally. Since you are
> > the first to add DT support please make separate patch to driver to drop
> > the ORQ trigger from request_theraded_irq() leaving only IRQF_ONESHOT.
> >
> This was simply an oversight on my part, my device was using the the polled
> method as opposed to an interrupt. I'll correct this in v2.
> > Also please create patch removing platform data support as noone is
> > using it upstream.
> >
> Will do.
> > What about the rest of config parameters from bma150_cfg? They should be
> > handled as device properties too.
> >
> Ok, I can add them as well. I didn't bother as the comment in the source
> code says that the default values are the ones recommended by Bosch.
OK, if you do not need to handle them then we can leave this task to the
next user ;)
Thanks.
--
Dmitry
Re: [PATCH 3/3] input: dt-bindings: Add binding for bma150 sensor
On 2019-01-25 5:28 p.m., Dmitry Torokhov wrote:
> On Fri, Jan 25, 2019 at 07:44:00PM +0100, Paweł Chmiel wrote:
>> From: Jonathan Bakker
>>
>> Add device tree bindings for Bosch BMA150 Accelerometer Sensor
>>
>> Signed-off-by: Jonathan Bakker
>> Signed-off-by: Paweł Chmiel
>> ---
>> .../bindings/input/bosch,bma150.txt | 20 +++
>> 1 file changed, 20 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/input/bosch,bma150.txt
>>
>> diff --git a/Documentation/devicetree/bindings/input/bosch,bma150.txt
>> b/Documentation/devicetree/bindings/input/bosch,bma150.txt
>> new file mode 100644
>> index ..290c60e38c70
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/input/bosch,bma150.txt
>> @@ -0,0 +1,20 @@
>> +* Bosch BMA150 Accelerometer Sensor
>> +
>> +Also works for the SMB380 and BMA023 accelerometers
>> +
>> +Required properties:
>> +- compatible : Should be "bosch,bma150"
>> +- reg : The I2C address of the sensor
>> +
>> +Optional properties:
>> +- interrupt-parent : should be the phandle for the interrupt controller
>> +- interrupts : Interrupt mapping for IRQ. If not present device will be
>> polled
>> +
>> +Example:
>> +
>> +bma150@38 {
>> +compatible = "bosch,bma150";
>> +reg = <0x38>;
>> +interrupt-parent = <>;
>> +interrupts = <1 IRQ_TYPE_LEVEL_HIGH>;
>
> Hmm, here you say that IRQ_TYPE_LEVEL_HIGH, so it is level interrupts,
> but the driver overrides to rising edge unconditionally. Since you are
> the first to add DT support please make separate patch to driver to drop
> the ORQ trigger from request_theraded_irq() leaving only IRQF_ONESHOT.
>
This was simply an oversight on my part, my device was using the the polled
method as opposed to an interrupt. I'll correct this in v2.
> Also please create patch removing platform data support as noone is
> using it upstream.
>
Will do.
> What about the rest of config parameters from bma150_cfg? They should be
> handled as device properties too.
>
Ok, I can add them as well. I didn't bother as the comment in the source code
says that the default values are the ones recommended by Bosch.
> Thanks.
>
Thanks,
Jonathan
Re: [PATCH 3/3] input: dt-bindings: Add binding for bma150 sensor
On Fri, Jan 25, 2019 at 07:44:00PM +0100, Paweł Chmiel wrote:
> From: Jonathan Bakker
>
> Add device tree bindings for Bosch BMA150 Accelerometer Sensor
>
> Signed-off-by: Jonathan Bakker
> Signed-off-by: Paweł Chmiel
> ---
> .../bindings/input/bosch,bma150.txt | 20 +++
> 1 file changed, 20 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/bosch,bma150.txt
>
> diff --git a/Documentation/devicetree/bindings/input/bosch,bma150.txt
> b/Documentation/devicetree/bindings/input/bosch,bma150.txt
> new file mode 100644
> index ..290c60e38c70
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/bosch,bma150.txt
> @@ -0,0 +1,20 @@
> +* Bosch BMA150 Accelerometer Sensor
> +
> +Also works for the SMB380 and BMA023 accelerometers
> +
> +Required properties:
> +- compatible : Should be "bosch,bma150"
> +- reg : The I2C address of the sensor
> +
> +Optional properties:
> +- interrupt-parent : should be the phandle for the interrupt controller
> +- interrupts : Interrupt mapping for IRQ. If not present device will be
> polled
> +
> +Example:
> +
> +bma150@38 {
> + compatible = "bosch,bma150";
> + reg = <0x38>;
> + interrupt-parent = <>;
> + interrupts = <1 IRQ_TYPE_LEVEL_HIGH>;
Hmm, here you say that IRQ_TYPE_LEVEL_HIGH, so it is level interrupts,
but the driver overrides to rising edge unconditionally. Since you are
the first to add DT support please make separate patch to driver to drop
the ORQ trigger from request_theraded_irq() leaving only IRQF_ONESHOT.
Also please create patch removing platform data support as noone is
using it upstream.
What about the rest of config parameters from bma150_cfg? They should be
handled as device properties too.
Thanks.
--
Dmitry
[PATCH 3/3] input: dt-bindings: Add binding for bma150 sensor
From: Jonathan Bakker
Add device tree bindings for Bosch BMA150 Accelerometer Sensor
Signed-off-by: Jonathan Bakker
Signed-off-by: Paweł Chmiel
---
.../bindings/input/bosch,bma150.txt | 20 +++
1 file changed, 20 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/bosch,bma150.txt
diff --git a/Documentation/devicetree/bindings/input/bosch,bma150.txt
b/Documentation/devicetree/bindings/input/bosch,bma150.txt
new file mode 100644
index ..290c60e38c70
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/bosch,bma150.txt
@@ -0,0 +1,20 @@
+* Bosch BMA150 Accelerometer Sensor
+
+Also works for the SMB380 and BMA023 accelerometers
+
+Required properties:
+- compatible : Should be "bosch,bma150"
+- reg : The I2C address of the sensor
+
+Optional properties:
+- interrupt-parent : should be the phandle for the interrupt controller
+- interrupts : Interrupt mapping for IRQ. If not present device will be polled
+
+Example:
+
+bma150@38 {
+ compatible = "bosch,bma150";
+ reg = <0x38>;
+ interrupt-parent = <>;
+ interrupts = <1 IRQ_TYPE_LEVEL_HIGH>;
+};
--
2.17.1
