diff mbox

[1/4] pinctrl: dt-bindings: samsung: add drive strength macros for Exynos5433

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

Commit Message

Andi Shyti Dec. 29, 2016, 8:42 a.m. UTC
Commit 5db7e3bb87df ("pinctrl: dt-bindings: samsung: Add header with
values used for configuration") has added a header file for defining the
pinctrl values in order to avoid hardcoded settings in the Exynos
DTS related files.

Extend samsung.h to the Exynos5433 for drive strength values
which are strictly related to the particular SoC and may defer
from others.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 include/dt-bindings/pinctrl/samsung.h | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Chanwoo Choi Dec. 29, 2016, 9:17 a.m. UTC | #1
Hi Andi,

On 2016년 12월 29일 17:42, Andi Shyti wrote:
> Commit 5db7e3bb87df ("pinctrl: dt-bindings: samsung: Add header with
> values used for configuration") has added a header file for defining the
> pinctrl values in order to avoid hardcoded settings in the Exynos
> DTS related files.
> 
> Extend samsung.h to the Exynos5433 for drive strength values
> which are strictly related to the particular SoC and may defer
> from others.
> 
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> ---
>  include/dt-bindings/pinctrl/samsung.h | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/include/dt-bindings/pinctrl/samsung.h b/include/dt-bindings/pinctrl/samsung.h
> index 6276eb785e2b..58868313d64b 100644
> --- a/include/dt-bindings/pinctrl/samsung.h
> +++ b/include/dt-bindings/pinctrl/samsung.h
> @@ -45,6 +45,12 @@
>  #define EXYNOS5420_PIN_DRV_LV3		2
>  #define EXYNOS5420_PIN_DRV_LV4		3
>  
> +/* Drive strengths for Exynos5433 */
> +#define EXYNOS5433_PIN_DRV_LV1		0
> +#define EXYNOS5433_PIN_DRV_LV2		1
> +#define EXYNOS5433_PIN_DRV_LV3		2
> +#define EXYNOS5433_PIN_DRV_LV4		3

Exynos5433 has the same value with EXYNOS5420. So, I'd like you to use the EXYNOS5420_PIN_DRV_LVx instead of separate the definitions.

> +
>  #define EXYNOS_PIN_FUNC_INPUT		0
>  #define EXYNOS_PIN_FUNC_OUTPUT		1
>  #define EXYNOS_PIN_FUNC_2		2
>
Jaehoon Chung Dec. 29, 2016, 9:39 a.m. UTC | #2
On 12/29/2016 05:42 PM, Andi Shyti wrote:
> Commit 5db7e3bb87df ("pinctrl: dt-bindings: samsung: Add header with
> values used for configuration") has added a header file for defining the
> pinctrl values in order to avoid hardcoded settings in the Exynos
> DTS related files.
> 
> Extend samsung.h to the Exynos5433 for drive strength values
> which are strictly related to the particular SoC and may defer
> from others.
> 
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> ---
>  include/dt-bindings/pinctrl/samsung.h | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/include/dt-bindings/pinctrl/samsung.h b/include/dt-bindings/pinctrl/samsung.h
> index 6276eb785e2b..58868313d64b 100644
> --- a/include/dt-bindings/pinctrl/samsung.h
> +++ b/include/dt-bindings/pinctrl/samsung.h
> @@ -45,6 +45,12 @@
>  #define EXYNOS5420_PIN_DRV_LV3		2
>  #define EXYNOS5420_PIN_DRV_LV4		3
>  
> +/* Drive strengths for Exynos5433 */
> +#define EXYNOS5433_PIN_DRV_LV1		0
> +#define EXYNOS5433_PIN_DRV_LV2		1
> +#define EXYNOS5433_PIN_DRV_LV3		2
> +#define EXYNOS5433_PIN_DRV_LV4		3

Well, i'm not sure..but you needs to compare the other Exynos5 series.
it's difference bit Offset. Did you check it?

I didn't check pinctrl file..if it doesn't apply any exynos5433 pinctrl for drv_strength.
it will work wrong..

Best Regards,
Jaehoon Chung

> +
>  #define EXYNOS_PIN_FUNC_INPUT		0
>  #define EXYNOS_PIN_FUNC_OUTPUT		1
>  #define EXYNOS_PIN_FUNC_2		2
> 
>
Chanwoo Choi Dec. 29, 2016, 10:33 a.m. UTC | #3
Hi Andi,

On 2016년 12월 29일 18:17, Chanwoo Choi wrote:
> Hi Andi,
> 
> On 2016년 12월 29일 17:42, Andi Shyti wrote:
>> Commit 5db7e3bb87df ("pinctrl: dt-bindings: samsung: Add header with
>> values used for configuration") has added a header file for defining the
>> pinctrl values in order to avoid hardcoded settings in the Exynos
>> DTS related files.
>>
>> Extend samsung.h to the Exynos5433 for drive strength values
>> which are strictly related to the particular SoC and may defer
>> from others.
>>
>> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
>> ---
>>  include/dt-bindings/pinctrl/samsung.h | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/include/dt-bindings/pinctrl/samsung.h b/include/dt-bindings/pinctrl/samsung.h
>> index 6276eb785e2b..58868313d64b 100644
>> --- a/include/dt-bindings/pinctrl/samsung.h
>> +++ b/include/dt-bindings/pinctrl/samsung.h
>> @@ -45,6 +45,12 @@
>>  #define EXYNOS5420_PIN_DRV_LV3		2
>>  #define EXYNOS5420_PIN_DRV_LV4		3
>>  
>> +/* Drive strengths for Exynos5433 */
>> +#define EXYNOS5433_PIN_DRV_LV1		0
>> +#define EXYNOS5433_PIN_DRV_LV2		1
>> +#define EXYNOS5433_PIN_DRV_LV3		2
>> +#define EXYNOS5433_PIN_DRV_LV4		3
> 
> Exynos5433 has the same value with EXYNOS5420. So, I'd like you to use the EXYNOS5420_PIN_DRV_LVx instead of separate the definitions.

I found the problem to handle the *_DRV register of Exynos5433. Because Exynos5433 has the different width length of *_DRV (PINCFG_TYPE_DRV) bitfields from Exynos542x as following. When I was sending the exynos5433 pinctrl patches, I was missing this issue.

Exynos5422/Exynos5410 have two different bitfields in the same register to set the DRV_LVx as following:
(n=0 to 7)
[2n+1:2n] : 2bits
 0x0 = 1x,
 0x1 = 2x,
 0x2 = 3x,
 0x3 = 4x,

[n+16:16]
 0x0 = Fast Slew Rate,
 0x1 = Slow Slew Rate,

But, Exynos5433 has the following value for PIN_DRV_LVx without additional bitfields to separate 'Fast Slew Rate' and 'Slow Slew Rate'. Just exynos5433 defines the 'Fast Slew Rate(0x0 ~ 0x5)' and 'Slow Slew Rate (0x8 ~ 0xF)'.
(n=0 to 7)
[4n+3:4n] : 4 bits
0x0 = Fast Slew Rate 1x
0x1 = Fast Slew Rate 2x
0x2 = Fast Slew Rate 3x
0x3 = Fast Slew Rate 4x
0x4 = Fast Slew Rate 5x
0x5 = Fast Slew Rate 6x
0x8 = Slow Slew Rate 1x
0x9 = Slow Slew Rate 2x
0xA = Slow Slew Rate 3x
0xB = Slow Slew Rate 4x
0xC = Slow Slew Rate 5x
0xF = Slow Slew Rate 6x

So, before this patch, we have to fix it to support the DRV reigster of Exynos5433.
I'll fix it.

> 
>> +
>>  #define EXYNOS_PIN_FUNC_INPUT		0
>>  #define EXYNOS_PIN_FUNC_OUTPUT		1
>>  #define EXYNOS_PIN_FUNC_2		2
>>
>
Krzysztof Kozlowski Dec. 29, 2016, 11:50 a.m. UTC | #4
On Thu, Dec 29, 2016 at 05:42:08PM +0900, Andi Shyti wrote:
> Commit 5db7e3bb87df ("pinctrl: dt-bindings: samsung: Add header with
> values used for configuration") has added a header file for defining the
> pinctrl values in order to avoid hardcoded settings in the Exynos
> DTS related files.
> 
> Extend samsung.h to the Exynos5433 for drive strength values
> which are strictly related to the particular SoC and may defer
> from others.
> 
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> ---
>  include/dt-bindings/pinctrl/samsung.h | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/include/dt-bindings/pinctrl/samsung.h b/include/dt-bindings/pinctrl/samsung.h
> index 6276eb785e2b..58868313d64b 100644
> --- a/include/dt-bindings/pinctrl/samsung.h
> +++ b/include/dt-bindings/pinctrl/samsung.h
> @@ -45,6 +45,12 @@
>  #define EXYNOS5420_PIN_DRV_LV3		2
>  #define EXYNOS5420_PIN_DRV_LV4		3
>  
> +/* Drive strengths for Exynos5433 */
> +#define EXYNOS5433_PIN_DRV_LV1		0
> +#define EXYNOS5433_PIN_DRV_LV2		1
> +#define EXYNOS5433_PIN_DRV_LV3		2
> +#define EXYNOS5433_PIN_DRV_LV4		3

Same values as existing. No need to re-define these.

Best regards,
Krzysztof
Andi Shyti Dec. 29, 2016, 1:41 p.m. UTC | #5
Hi Krzysztof,

> >  #define EXYNOS5420_PIN_DRV_LV3		2
> >  #define EXYNOS5420_PIN_DRV_LV4		3
> >  
> > +/* Drive strengths for Exynos5433 */
> > +#define EXYNOS5433_PIN_DRV_LV1		0
> > +#define EXYNOS5433_PIN_DRV_LV2		1
> > +#define EXYNOS5433_PIN_DRV_LV3		2
> > +#define EXYNOS5433_PIN_DRV_LV4		3
> 
> Same values as existing. No need to re-define these.

Yes, I was in doubt when I prepared this patch as on one hand it
didn't look right to use EXYNOS5420_* for exynos5433, on the other
hand it didn't look right to duplicate the macros.

In anycase this values need to be fixed as Chanwoo wrote in the
other mail.

Thanks,
Andi
diff mbox

Patch

diff --git a/include/dt-bindings/pinctrl/samsung.h b/include/dt-bindings/pinctrl/samsung.h
index 6276eb785e2b..58868313d64b 100644
--- a/include/dt-bindings/pinctrl/samsung.h
+++ b/include/dt-bindings/pinctrl/samsung.h
@@ -45,6 +45,12 @@ 
 #define EXYNOS5420_PIN_DRV_LV3		2
 #define EXYNOS5420_PIN_DRV_LV4		3
 
+/* Drive strengths for Exynos5433 */
+#define EXYNOS5433_PIN_DRV_LV1		0
+#define EXYNOS5433_PIN_DRV_LV2		1
+#define EXYNOS5433_PIN_DRV_LV3		2
+#define EXYNOS5433_PIN_DRV_LV4		3
+
 #define EXYNOS_PIN_FUNC_INPUT		0
 #define EXYNOS_PIN_FUNC_OUTPUT		1
 #define EXYNOS_PIN_FUNC_2		2