diff mbox

[v5,5/5] arm64: dts: exynos: Add tm2 touchkey node

Message ID 20170106134350.32428-6-andi.shyti@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andi Shyti Jan. 6, 2017, 1:43 p.m. UTC
From: Jaechul Lee <jcsing.lee@samsung.com>

Add DT node support for TM2 touchkey device.

Signed-off-by: Beomho Seo <beomho.seo@samsung.com>
Signed-off-by: Jaechul Lee <jcsing.lee@samsung.com>
Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
---
 arch/arm64/boot/dts/exynos/exynos5433-tm2.dts | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Krzysztof Kozlowski Jan. 6, 2017, 2:03 p.m. UTC | #1
On Fri, Jan 06, 2017 at 10:43:50PM +0900, Andi Shyti wrote:
> From: Jaechul Lee <jcsing.lee@samsung.com>
> 
> Add DT node support for TM2 touchkey device.
> 
> Signed-off-by: Beomho Seo <beomho.seo@samsung.com>
> Signed-off-by: Jaechul Lee <jcsing.lee@samsung.com>
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
> ---
>  arch/arm64/boot/dts/exynos/exynos5433-tm2.dts | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 

Looks good, but I will wait with this one till the bindings and driver
got accepted.

Best regards,
Krzysztof
Chanwoo Choi Jan. 6, 2017, 9:31 p.m. UTC | #2
Hi,

2017-01-06 22:43 GMT+09:00 Andi Shyti <andi.shyti@samsung.com>:
> From: Jaechul Lee <jcsing.lee@samsung.com>
>
> Add DT node support for TM2 touchkey device.
>
> Signed-off-by: Beomho Seo <beomho.seo@samsung.com>
> Signed-off-by: Jaechul Lee <jcsing.lee@samsung.com>
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
> ---
>  arch/arm64/boot/dts/exynos/exynos5433-tm2.dts | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> index 2449266b268f..92fcc4ec8319 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> @@ -18,6 +18,19 @@
>         compatible = "samsung,tm2", "samsung,exynos5433";
>  };
>
> +&hsi2c_9 {
> +       status = "okay";
> +
> +       touchkey@20 {
> +               compatible = "samsung,tm2-touchkey";
> +               reg = <0x20>;
> +               interrupt-parent = <&gpa3>;
> +               interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
> +               vcc-supply = <&ldo32_reg>;
> +               vdd-supply = <&ldo33_reg>;
> +       };
> +};
> +
>  &ldo31_reg {
>         regulator-name = "TSP_VDD_1.85V_AP";
>         regulator-min-microvolt = <1850000>;

Looks good to me.
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
Chanwoo Choi Jan. 6, 2017, 10:09 p.m. UTC | #3
Hi,

2017-01-07 6:31 GMT+09:00 Chanwoo Choi <cwchoi00@gmail.com>:
> Hi,
>
> 2017-01-06 22:43 GMT+09:00 Andi Shyti <andi.shyti@samsung.com>:
>> From: Jaechul Lee <jcsing.lee@samsung.com>
>>
>> Add DT node support for TM2 touchkey device.
>>
>> Signed-off-by: Beomho Seo <beomho.seo@samsung.com>
>> Signed-off-by: Jaechul Lee <jcsing.lee@samsung.com>
>> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
>> Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
>> ---
>>  arch/arm64/boot/dts/exynos/exynos5433-tm2.dts | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
>> index 2449266b268f..92fcc4ec8319 100644
>> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
>> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
>> @@ -18,6 +18,19 @@
>>         compatible = "samsung,tm2", "samsung,exynos5433";
>>  };
>>
>> +&hsi2c_9 {
>> +       status = "okay";
>> +
>> +       touchkey@20 {
>> +               compatible = "samsung,tm2-touchkey";
>> +               reg = <0x20>;
>> +               interrupt-parent = <&gpa3>;
>> +               interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
>> +               vcc-supply = <&ldo32_reg>;
>> +               vdd-supply = <&ldo33_reg>;
>> +       };
>> +};
>> +
>>  &ldo31_reg {
>>         regulator-name = "TSP_VDD_1.85V_AP";
>>         regulator-min-microvolt = <1850000>;
>
> Looks good to me.
> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
>
> --
> Best Regards,
> Chanwoo Choi
Chanwoo Choi Jan. 6, 2017, 10:15 p.m. UTC | #4
Hi,

2017-01-07 7:09 GMT+09:00 Chanwoo Choi <cwchoi00@gmail.com>:
> Hi,
>
> 2017-01-07 6:31 GMT+09:00 Chanwoo Choi <cwchoi00@gmail.com>:
>> Hi,
>>
>> 2017-01-06 22:43 GMT+09:00 Andi Shyti <andi.shyti@samsung.com>:
>>> From: Jaechul Lee <jcsing.lee@samsung.com>
>>>
>>> Add DT node support for TM2 touchkey device.
>>>
>>> Signed-off-by: Beomho Seo <beomho.seo@samsung.com>
>>> Signed-off-by: Jaechul Lee <jcsing.lee@samsung.com>
>>> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
>>> Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
>>> ---
>>>  arch/arm64/boot/dts/exynos/exynos5433-tm2.dts | 13 +++++++++++++
>>>  1 file changed, 13 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
>>> index 2449266b268f..92fcc4ec8319 100644
>>> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
>>> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
>>> @@ -18,6 +18,19 @@
>>>         compatible = "samsung,tm2", "samsung,exynos5433";
>>>  };
>>>
>>> +&hsi2c_9 {
>>> +       status = "okay";
>>> +
>>> +       touchkey@20 {
>>> +               compatible = "samsung,tm2-touchkey";
>>> +               reg = <0x20>;
>>> +               interrupt-parent = <&gpa3>;
>>> +               interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
>>> +               vcc-supply = <&ldo32_reg>;
>>> +               vdd-supply = <&ldo33_reg>;
>>> +       };
>>> +};
>>> +
>>>  &ldo31_reg {
>>>         regulator-name = "TSP_VDD_1.85V_AP";
>>>         regulator-min-microvolt = <1850000>;
>>
>> Looks good to me.
>> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>

I'm sorry to send the empty mail without any content.

I repiled the touchkey driver against compatible name.
Usually, when developing the device driver, we use the specific h/w name
but in this patch, the touckey dt node uses the h/w board name instead of
original touckhey name.

If this touckey device is used on other h/w board, I think the current
compatible name
is not proper.

So. please drop the my reviewed-by tag until completing the discussion.
I'm sorry to make the confusion.
Andi Shyti Jan. 7, 2017, 12:40 p.m. UTC | #5
Hi Chanwoo,

> >>> +       touchkey@20 {
> >>> +               compatible = "samsung,tm2-touchkey";
> >>> +               reg = <0x20>;
> >>> +               interrupt-parent = <&gpa3>;
> >>> +               interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
> >>> +               vcc-supply = <&ldo32_reg>;
> >>> +               vdd-supply = <&ldo33_reg>;
> >>> +       };
> >>> +};
> >>> +
> >>>  &ldo31_reg {
> >>>         regulator-name = "TSP_VDD_1.85V_AP";
> >>>         regulator-min-microvolt = <1850000>;
> 
> I repiled the touchkey driver against compatible name.
> Usually, when developing the device driver, we use the specific h/w name
> but in this patch, the touckey dt node uses the h/w board name instead of
> original touckhey name.

this should be a device specifically done for the tm2 and we are
not sure who is the manufacturer of the device. In order to not
assign the device to the wrong manufacturer, we preferred calling
it Samsung as it is in a Samsung device.

Andi
Chanwoo Choi Jan. 8, 2017, 5:14 a.m. UTC | #6
Hi Andi,

2017-01-07 21:40 GMT+09:00 Andi Shyti <andi@etezian.org>:
> Hi Chanwoo,
>
>> >>> +       touchkey@20 {
>> >>> +               compatible = "samsung,tm2-touchkey";
>> >>> +               reg = <0x20>;
>> >>> +               interrupt-parent = <&gpa3>;
>> >>> +               interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
>> >>> +               vcc-supply = <&ldo32_reg>;
>> >>> +               vdd-supply = <&ldo33_reg>;
>> >>> +       };
>> >>> +};
>> >>> +
>> >>>  &ldo31_reg {
>> >>>         regulator-name = "TSP_VDD_1.85V_AP";
>> >>>         regulator-min-microvolt = <1850000>;
>>
>> I repiled the touchkey driver against compatible name.
>> Usually, when developing the device driver, we use the specific h/w name
>> but in this patch, the touckey dt node uses the h/w board name instead of
>> original touckhey name.
>
> this should be a device specifically done for the tm2 and we are
> not sure who is the manufacturer of the device. In order to not

As I knew, this touchkey was made by Cypress semiconductor.
But, for more correct information, you may try to find it.

> assign the device to the wrong manufacturer, we preferred calling
> it Samsung as it is in a Samsung device.

As you mentioned, Samsung made not this touchkey device. It is
certainly wrong manufacturer. I have not seen to use the h/w board
name as the peripheral device name.

I don't prefer to use the inaccurate manufacturer and device name.
Andi Shyti Jan. 8, 2017, 5:45 a.m. UTC | #7
Hi Chanwoo,

> >> >>> +       touchkey@20 {
> >> >>> +               compatible = "samsung,tm2-touchkey";
> >> >>> +               reg = <0x20>;
> >> >>> +               interrupt-parent = <&gpa3>;
> >> >>> +               interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
> >> >>> +               vcc-supply = <&ldo32_reg>;
> >> >>> +               vdd-supply = <&ldo33_reg>;
> >> >>> +       };
> >> >>> +};
> >> >>> +
> >> >>>  &ldo31_reg {
> >> >>>         regulator-name = "TSP_VDD_1.85V_AP";
> >> >>>         regulator-min-microvolt = <1850000>;
> >>
> >> I repiled the touchkey driver against compatible name.
> >> Usually, when developing the device driver, we use the specific h/w name
> >> but in this patch, the touckey dt node uses the h/w board name instead of
> >> original touckhey name.
> >
> > this should be a device specifically done for the tm2 and we are
> > not sure who is the manufacturer of the device. In order to not
> 
> As I knew, this touchkey was made by Cypress semiconductor.
> But, for more correct information, you may try to find it.

The Android Kernel says that this is the cy8cmbr3xxx. I
downloaded the datasheets, but it doesn't have any similarity
with the device. Which means that on tm2 we don't the
cy8cmbr3xxx bot something else that I even doubt comes from
cypress, but I strongly believe it's a dedicated hardware.

Now we have two choices:

1. drop support because we are not 100% sure on the device and
supplier.

2. call it Samsung and provide support anyway as, at the end, it
is a samsung hardware.

With Jaechul we chose option 2.

> > assign the device to the wrong manufacturer, we preferred calling
> > it Samsung as it is in a Samsung device.
> 
> As you mentioned, Samsung made not this touchkey device. It is
> certainly wrong manufacturer. I have not seen to use the h/w board
> name as the peripheral device name.
> 
> I don't prefer to use the inaccurate manufacturer and device name.

Eventually, Jaechul can assign the device to cypress, but the
name has to remain tm2-touchkey. Something like this:

	compatible = "cypress,tm2-touchkey";

What do you think?

Andi
Chanwoo Choi Jan. 8, 2017, 6:07 a.m. UTC | #8
Hi Andi,

2017-01-08 14:45 GMT+09:00 Andi Shyti <andi@etezian.org>:
> Hi Chanwoo,
>
>> >> >>> +       touchkey@20 {
>> >> >>> +               compatible = "samsung,tm2-touchkey";
>> >> >>> +               reg = <0x20>;
>> >> >>> +               interrupt-parent = <&gpa3>;
>> >> >>> +               interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
>> >> >>> +               vcc-supply = <&ldo32_reg>;
>> >> >>> +               vdd-supply = <&ldo33_reg>;
>> >> >>> +       };
>> >> >>> +};
>> >> >>> +
>> >> >>>  &ldo31_reg {
>> >> >>>         regulator-name = "TSP_VDD_1.85V_AP";
>> >> >>>         regulator-min-microvolt = <1850000>;
>> >>
>> >> I repiled the touchkey driver against compatible name.
>> >> Usually, when developing the device driver, we use the specific h/w name
>> >> but in this patch, the touckey dt node uses the h/w board name instead of
>> >> original touckhey name.
>> >
>> > this should be a device specifically done for the tm2 and we are
>> > not sure who is the manufacturer of the device. In order to not
>>
>> As I knew, this touchkey was made by Cypress semiconductor.
>> But, for more correct information, you may try to find it.
>
> The Android Kernel says that this is the cy8cmbr3xxx. I
> downloaded the datasheets, but it doesn't have any similarity
> with the device. Which means that on tm2 we don't the
> cy8cmbr3xxx bot something else that I even doubt comes from
> cypress, but I strongly believe it's a dedicated hardware.

Although this device is a dedicated h/w, you should keep the principal.
Did you check the schematic document of TM2? The schematic document
might include the specific device name.

Sometimes, the downloaded datasheet does not include the
all of information of device because of confidential information.
But, I’m not sure. Just it is guess as my experience.

I want to check the manufacturer and name of device with you
on next Monday.

>
> Now we have two choices:
>
> 1. drop support because we are not 100% sure on the device and
> supplier.
>
> 2. call it Samsung and provide support anyway as, at the end, it
> is a samsung hardware.
>
> With Jaechul we chose option 2.
>
>> > assign the device to the wrong manufacturer, we preferred calling
>> > it Samsung as it is in a Samsung device.
>>
>> As you mentioned, Samsung made not this touchkey device. It is
>> certainly wrong manufacturer. I have not seen to use the h/w board
>> name as the peripheral device name.
>>
>> I don't prefer to use the inaccurate manufacturer and device name.
>
> Eventually, Jaechul can assign the device to cypress, but the
> name has to remain tm2-touchkey. Something like this:
>
>         compatible = "cypress,tm2-touchkey";
>
> What do you think?

If we never to find the correct device name, we might use your suggestion.
As I already said, I’d like to check it again with you.
Andi Shyti Jan. 8, 2017, 6:13 a.m. UTC | #9
Hi Chanwoo,

> >> >> >>> +       touchkey@20 {
> >> >> >>> +               compatible = "samsung,tm2-touchkey";
> >> >> >>> +               reg = <0x20>;
> >> >> >>> +               interrupt-parent = <&gpa3>;
> >> >> >>> +               interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
> >> >> >>> +               vcc-supply = <&ldo32_reg>;
> >> >> >>> +               vdd-supply = <&ldo33_reg>;
> >> >> >>> +       };
> >> >> >>> +};
> >> >> >>> +
> >> >> >>>  &ldo31_reg {
> >> >> >>>         regulator-name = "TSP_VDD_1.85V_AP";
> >> >> >>>         regulator-min-microvolt = <1850000>;
> >> >>
> >> >> I repiled the touchkey driver against compatible name.
> >> >> Usually, when developing the device driver, we use the specific h/w name
> >> >> but in this patch, the touckey dt node uses the h/w board name instead of
> >> >> original touckhey name.
> >> >
> >> > this should be a device specifically done for the tm2 and we are
> >> > not sure who is the manufacturer of the device. In order to not
> >>
> >> As I knew, this touchkey was made by Cypress semiconductor.
> >> But, for more correct information, you may try to find it.
> >
> > The Android Kernel says that this is the cy8cmbr3xxx. I
> > downloaded the datasheets, but it doesn't have any similarity
> > with the device. Which means that on tm2 we don't the
> > cy8cmbr3xxx bot something else that I even doubt comes from
> > cypress, but I strongly believe it's a dedicated hardware.
> 
> Although this device is a dedicated h/w, you should keep the principal.
> Did you check the schematic document of TM2? The schematic document
> might include the specific device name.
> 
> Sometimes, the downloaded datasheet does not include the
> all of information of device because of confidential information.
> But, I’m not sure. Just it is guess as my experience.
> 
> I want to check the manufacturer and name of device with you
> on next Monday.

Yes, I checked everything, but didn't find the exact device name.

Anyway, let's take this discussion offline and find the correct
way.

Andi
diff mbox

Patch

diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
index 2449266b268f..92fcc4ec8319 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
+++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
@@ -18,6 +18,19 @@ 
 	compatible = "samsung,tm2", "samsung,exynos5433";
 };
 
+&hsi2c_9 {
+	status = "okay";
+
+	touchkey@20 {
+		compatible = "samsung,tm2-touchkey";
+		reg = <0x20>;
+		interrupt-parent = <&gpa3>;
+		interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
+		vcc-supply = <&ldo32_reg>;
+		vdd-supply = <&ldo33_reg>;
+	};
+};
+
 &ldo31_reg {
 	regulator-name = "TSP_VDD_1.85V_AP";
 	regulator-min-microvolt = <1850000>;