Re: [PATCH v2] pinctrl: rockchip: do coding style for mux route struct
Am Dienstag, 20. April 2021, 11:07:27 CEST schrieb Jianqun Xu:
> The mux route tables take many lines for each SoC, and it will be more
> instances for newly SoC, that makes the file size increase larger.
>
> This patch only do coding style for mux route struct, by adding a new
> definition and replace the structs by script which supplied by
> huang...@rock-chips.com
>
> sed -i -e "
> /static struct rockchip_mux_route_data /bcheck
> b
> :append-next-line
> N
> :check
> /^[^;]*$/bappend-next-line
> s/[[:blank:]]*.bank_num = \([[:digit:]]*,\)\n/\tRK_MUXROUTE_SAME(\1/g
> s/[[:blank:]]*.pin =[[:blank:]]*0,\n/ RK_PA0,/g
> s/[[:blank:]]*.pin =[[:blank:]]*1,\n/ RK_PA1,/g
> s/[[:blank:]]*.pin =[[:blank:]]*2,\n/ RK_PA2,/g
> s/[[:blank:]]*.pin =[[:blank:]]*3,\n/ RK_PA3,/g
> s/[[:blank:]]*.pin =[[:blank:]]*4,\n/ RK_PA4,/g
> s/[[:blank:]]*.pin =[[:blank:]]*5,\n/ RK_PA5,/g
> s/[[:blank:]]*.pin =[[:blank:]]*6,\n/ RK_PA6,/g
> s/[[:blank:]]*.pin =[[:blank:]]*7,\n/ RK_PA7,/g
> s/[[:blank:]]*.pin =[[:blank:]]*8,\n/ RK_PB0,/g
> s/[[:blank:]]*.pin =[[:blank:]]*9,\n/ RK_PB1,/g
> s/[[:blank:]]*.pin =[[:blank:]]*10,\n/ RK_PB2,/g
> s/[[:blank:]]*.pin =[[:blank:]]*11,\n/ RK_PB3,/g
> s/[[:blank:]]*.pin =[[:blank:]]*12,\n/ RK_PB4,/g
> s/[[:blank:]]*.pin =[[:blank:]]*13,\n/ RK_PB5,/g
> s/[[:blank:]]*.pin =[[:blank:]]*14,\n/ RK_PB6,/g
> s/[[:blank:]]*.pin =[[:blank:]]*15,\n/ RK_PB7,/g
> s/[[:blank:]]*.pin =[[:blank:]]*16,\n/ RK_PC0,/g
> s/[[:blank:]]*.pin =[[:blank:]]*17,\n/ RK_PC1,/g
> s/[[:blank:]]*.pin =[[:blank:]]*18,\n/ RK_PC2,/g
> s/[[:blank:]]*.pin =[[:blank:]]*19,\n/ RK_PC3,/g
> s/[[:blank:]]*.pin =[[:blank:]]*20,\n/ RK_PC4,/g
> s/[[:blank:]]*.pin =[[:blank:]]*21,\n/ RK_PC5,/g
> s/[[:blank:]]*.pin =[[:blank:]]*22,\n/ RK_PC6,/g
> s/[[:blank:]]*.pin =[[:blank:]]*23,\n/ RK_PC7,/g
> s/[[:blank:]]*.pin =[[:blank:]]*24,\n/ RK_PD0,/g
> s/[[:blank:]]*.pin =[[:blank:]]*25,\n/ RK_PD1,/g
> s/[[:blank:]]*.pin =[[:blank:]]*26,\n/ RK_PD2,/g
> s/[[:blank:]]*.pin =[[:blank:]]*27,\n/ RK_PD3,/g
> s/[[:blank:]]*.pin =[[:blank:]]*28,\n/ RK_PD4,/g
> s/[[:blank:]]*.pin =[[:blank:]]*29,\n/ RK_PD5,/g
> s/[[:blank:]]*.pin =[[:blank:]]*30,\n/ RK_PD6,/g
> s/[[:blank:]]*.pin =[[:blank:]]*31,\n/ RK_PD7,/g
> s/[[:blank:]]*.func = \([[:digit:]]*,\)\n/ \1/g
> s/[[:blank:]]*.route_location =[[:blank:]]*\([[:print:]]*,\)\n//g
> s/[[:blank:]]*.route_offset = \(0x[[:xdigit:]]*,\)\n/ \1/g
> s/[[:blank:]]*.route_val =[[:blank:]]*\([[:print:]]*\),\n/ \1),/g
> s/\t{\n//g
> s/\t}, {\n//g
> s/\t},//g
> s/[[:blank:]]*\(\/\*[[:print:]]*\*\/\)\n[[:blank:]]*RK_MUXROUTE_SAME(\([[:print:]]*\)),\n/\tRK_MUXROUTE_SAME(\2),
> \1\n/g
> s/[[:blank:]]*\(\/\*[[:print:]]*\*\/\)\n[[:blank:]]*RK_MUXROUTE_SAME(\([[:print:]]*\)),/\tRK_MUXROUTE_SAME(\2),
> \1\n/g
> " drivers/pinctrl/pinctrl-rockchip.c
>
> Signed-off-by: Jianqun Xu
> Reviewed-by: Heiko Stuebner
> Change-Id: I50a12f00ae127df8938221d10f94e6733baab1c5
Looks like you still have that gerrit hook in your git tree, so
it inserted a new Change-Id :-(
Heiko
> ---
> v2:
> - remove change-id
> - reviewed-by heiko
>
> drivers/pinctrl/pinctrl-rockchip.c | 650 -
> 1 file changed, 80 insertions(+), 570 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-rockchip.c
> b/drivers/pinctrl/pinctrl-rockchip.c
> index aa1a1c850d05..c135ea847c54 100644
> --- a/drivers/pinctrl/pinctrl-rockchip.c
> +++ b/drivers/pinctrl/pinctrl-rockchip.c
> @@ -801,597 +801,107 @@ static void rockchip_get_recalced_mux(struct
> rockchip_pin_bank *bank, int pin,
> }
>
> static struct rockchip_mux_route_data px30_mux_route_data[] = {
> - {
> - /* cif-d2m0 */
> - .bank_num = 2,
> - .pin = 0,
> - .func = 1,
> - .route_offset = 0x184,
> - .route_val = BIT(16 + 7),
> - }, {
> - /* cif-d2m1 */
> - .bank_num = 3,
> - .pin = 3,
> - .func = 3,
> - .route_offset = 0x184,
> - .route_val = BIT(16 + 7) | BIT(7),
> - }, {
> - /* pdm-m0 */
> - .bank_num = 3,
> - .pin = 22,
> - .func = 2,
> - .route_offset = 0x184,
> - .route_val = BIT(16 + 8),
> - }, {
> - /* pdm-m1 */
> - .bank_num = 2,
> - .pin = 22,
> - .func = 1,
> - .route_offset = 0x184,
> - .route_val = BIT(16 + 8) | BIT(8),
> - }, {
> - /* uart2-rxm0 */
> - .bank_num = 1,
> - .pin = 27,
> - .func = 2,
> - .route_offset = 0x184,
> - .route_val = BIT(16 + 10),
> - }, {
> - /* uart2-rxm1 */
> - .bank_num = 2,
> - .pin = 14,
> - .func = 2,
> - .route_offset = 0x184,
> - .route_val = BIT(16 + 10) | BIT(10),
> - }, {
> - /* uart3-rxm0 */
> - .bank_num = 0,
> - .pin = 17,
> -
Re: [PATCH] pinctrl: rockchip: do coding style for mux route struct
Hi Jay,
Am Dienstag, 20. April 2021, 10:41:34 CEST schrieb jay...@rock-chips.com:
> Do I need to send v2 with change-id abandon ?
do everything to make a Maintainer's life easier :-) .
So I guess yes and of course try to make sure it applies to the
pinctrl devel branch
[
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/log/?h=devel
]
and add the necessary versioning information, so that people see
which one is newer.
Heiko
> From: HeikoStübner
> Date: 2021-04-20 15:35
> To: linus.walleij; Jianqun Xu
> CC: huangtao; linux-gpio; linux-rockchip; linux-kernel; Jianqun Xu
> Subject: Re: [PATCH] pinctrl: rockchip: do coding style for mux route struct
> Hi,
>
> Am Dienstag, 6. April 2021, 04:53:56 CEST schrieb Jianqun Xu:
> > The mux route tables take many lines for each SoC, and it will be more
> > instances for newly SoC, that makes the file size increase larger.
> >
> > This patch only do coding style for mux route struct, by adding a new
> > definition and replace the structs by script which supplied by
> > huang...@rock-chips.com
> >
> > sed -i -e "
> > /static struct rockchip_mux_route_data /bcheck
> > b
> > :append-next-line
> > N
> > :check
> > /^[^;]*$/bappend-next-line
> > s/[[:blank:]]*.bank_num = \([[:digit:]]*,\)\n/\tRK_MUXROUTE_SAME(\1/g
> > s/[[:blank:]]*.pin =[[:blank:]]*0,\n/ RK_PA0,/g
> > s/[[:blank:]]*.pin =[[:blank:]]*1,\n/ RK_PA1,/g
> > s/[[:blank:]]*.pin =[[:blank:]]*2,\n/ RK_PA2,/g
> > s/[[:blank:]]*.pin =[[:blank:]]*3,\n/ RK_PA3,/g
> > s/[[:blank:]]*.pin =[[:blank:]]*4,\n/ RK_PA4,/g
> > s/[[:blank:]]*.pin =[[:blank:]]*5,\n/ RK_PA5,/g
> > s/[[:blank:]]*.pin =[[:blank:]]*6,\n/ RK_PA6,/g
> > s/[[:blank:]]*.pin =[[:blank:]]*7,\n/ RK_PA7,/g
> > s/[[:blank:]]*.pin =[[:blank:]]*8,\n/ RK_PB0,/g
> > s/[[:blank:]]*.pin =[[:blank:]]*9,\n/ RK_PB1,/g
> > s/[[:blank:]]*.pin =[[:blank:]]*10,\n/ RK_PB2,/g
> > s/[[:blank:]]*.pin =[[:blank:]]*11,\n/ RK_PB3,/g
> > s/[[:blank:]]*.pin =[[:blank:]]*12,\n/ RK_PB4,/g
> > s/[[:blank:]]*.pin =[[:blank:]]*13,\n/ RK_PB5,/g
> > s/[[:blank:]]*.pin =[[:blank:]]*14,\n/ RK_PB6,/g
> > s/[[:blank:]]*.pin =[[:blank:]]*15,\n/ RK_PB7,/g
> > s/[[:blank:]]*.pin =[[:blank:]]*16,\n/ RK_PC0,/g
> > s/[[:blank:]]*.pin =[[:blank:]]*17,\n/ RK_PC1,/g
> > s/[[:blank:]]*.pin =[[:blank:]]*18,\n/ RK_PC2,/g
> > s/[[:blank:]]*.pin =[[:blank:]]*19,\n/ RK_PC3,/g
> > s/[[:blank:]]*.pin =[[:blank:]]*20,\n/ RK_PC4,/g
> > s/[[:blank:]]*.pin =[[:blank:]]*21,\n/ RK_PC5,/g
> > s/[[:blank:]]*.pin =[[:blank:]]*22,\n/ RK_PC6,/g
> > s/[[:blank:]]*.pin =[[:blank:]]*23,\n/ RK_PC7,/g
> > s/[[:blank:]]*.pin =[[:blank:]]*24,\n/ RK_PD0,/g
> > s/[[:blank:]]*.pin =[[:blank:]]*25,\n/ RK_PD1,/g
> > s/[[:blank:]]*.pin =[[:blank:]]*26,\n/ RK_PD2,/g
> > s/[[:blank:]]*.pin =[[:blank:]]*27,\n/ RK_PD3,/g
> > s/[[:blank:]]*.pin =[[:blank:]]*28,\n/ RK_PD4,/g
> > s/[[:blank:]]*.pin =[[:blank:]]*29,\n/ RK_PD5,/g
> > s/[[:blank:]]*.pin =[[:blank:]]*30,\n/ RK_PD6,/g
> > s/[[:blank:]]*.pin =[[:blank:]]*31,\n/ RK_PD7,/g
> > s/[[:blank:]]*.func = \([[:digit:]]*,\)\n/ \1/g
> > s/[[:blank:]]*.route_location =[[:blank:]]*\([[:print:]]*,\)\n//g
> > s/[[:blank:]]*.route_offset = \(0x[[:xdigit:]]*,\)\n/ \1/g
> > s/[[:blank:]]*.route_val =[[:blank:]]*\([[:print:]]*\),\n/ \1),/g
> > s/\t{\n//g
> > s/\t}, {\n//g
> > s/\t},//g
> > s/[[:blank:]]*\(\/\*[[:print:]]*\*\/\)\n[[:blank:]]*RK_MUXROUTE_SAME(\([[:print:]]*\)),\n/\tRK_MUXROUTE_SAME(\2),
> > \1\n/g
> > s/[[:blank:]]*\(\/\*[[:print:]]*\*\/\)\n[[:blank:]]*RK_MUXROUTE_SAME(\([[:print:]]*\)),/\tRK_MUXROUTE_SAME(\2),
> > \1\n/g
> > " drivers/pinctrl/pinctrl-rockchip.c
> >
> > Signed-off-by: Jianqun Xu
> > Change-Id: Ifc823d9557605b9dfcc9c0455a739f04f3fce5be
>
> Change-id should not be in here.
>
> Somehow I remember giving this a "reviewed-by" before in some
> previous version, but my memory may be faulty, so
>
> Reviewed-by: Heiko Stuebner
>
> and of course I like this change very much ;-)
>
>
> Though it may be really helpful to not send these individual patches but
> instead make it part of the series and put it at the beginning.
> Tracking the order patches should get applied when they're sent individually
> can get very hard.
>
> Heiko
>
>
> > ---
> > drivers/pinctrl/pinctrl-rockchip.c | 669 +
> > 1 file changed, 99 insertions(+), 570 deletions(-)
> >
> > diff --git a/drivers/pinctrl/pinctrl-rockchip.c
> > b/drivers/pinctrl/pinctrl-rockchip.c
> > index deabfbc74a01..6ba31c66ef8b 100644
> > --- a/drivers/pinctrl/pinctrl-rockchip.c
> > +++ b/drivers/pinctrl/pinctrl-rockchip.c
> > @@ -292,6 +292,25 @@ struct rockchip_pin_bank {
> > .pull_type[3] = pull3, \
> > }
> >
> > +#define PIN_BANK_MUX_ROUTE_FLAGS(ID, PIN, FUNC, REG, VAL, FLAG) \
> > + { \
> > + .bank_num = ID, \
> > + .pin = PIN, \
> > + .func = FUNC, \
> > + .route_offset = REG, \
> > + .route_val = VAL, \
> > + .route_location = FLAG, \
> > + }
> > +
> > +#define RK_MUXROUTE_SAME(ID, PIN, FUNC, REG, VAL) \
> > +
Re: [PATCH] pinctrl: rockchip: do coding style for mux route struct
Hi,
Am Dienstag, 6. April 2021, 04:53:56 CEST schrieb Jianqun Xu:
> The mux route tables take many lines for each SoC, and it will be more
> instances for newly SoC, that makes the file size increase larger.
>
> This patch only do coding style for mux route struct, by adding a new
> definition and replace the structs by script which supplied by
> huang...@rock-chips.com
>
> sed -i -e "
> /static struct rockchip_mux_route_data /bcheck
> b
> :append-next-line
> N
> :check
> /^[^;]*$/bappend-next-line
> s/[[:blank:]]*.bank_num = \([[:digit:]]*,\)\n/\tRK_MUXROUTE_SAME(\1/g
> s/[[:blank:]]*.pin =[[:blank:]]*0,\n/ RK_PA0,/g
> s/[[:blank:]]*.pin =[[:blank:]]*1,\n/ RK_PA1,/g
> s/[[:blank:]]*.pin =[[:blank:]]*2,\n/ RK_PA2,/g
> s/[[:blank:]]*.pin =[[:blank:]]*3,\n/ RK_PA3,/g
> s/[[:blank:]]*.pin =[[:blank:]]*4,\n/ RK_PA4,/g
> s/[[:blank:]]*.pin =[[:blank:]]*5,\n/ RK_PA5,/g
> s/[[:blank:]]*.pin =[[:blank:]]*6,\n/ RK_PA6,/g
> s/[[:blank:]]*.pin =[[:blank:]]*7,\n/ RK_PA7,/g
> s/[[:blank:]]*.pin =[[:blank:]]*8,\n/ RK_PB0,/g
> s/[[:blank:]]*.pin =[[:blank:]]*9,\n/ RK_PB1,/g
> s/[[:blank:]]*.pin =[[:blank:]]*10,\n/ RK_PB2,/g
> s/[[:blank:]]*.pin =[[:blank:]]*11,\n/ RK_PB3,/g
> s/[[:blank:]]*.pin =[[:blank:]]*12,\n/ RK_PB4,/g
> s/[[:blank:]]*.pin =[[:blank:]]*13,\n/ RK_PB5,/g
> s/[[:blank:]]*.pin =[[:blank:]]*14,\n/ RK_PB6,/g
> s/[[:blank:]]*.pin =[[:blank:]]*15,\n/ RK_PB7,/g
> s/[[:blank:]]*.pin =[[:blank:]]*16,\n/ RK_PC0,/g
> s/[[:blank:]]*.pin =[[:blank:]]*17,\n/ RK_PC1,/g
> s/[[:blank:]]*.pin =[[:blank:]]*18,\n/ RK_PC2,/g
> s/[[:blank:]]*.pin =[[:blank:]]*19,\n/ RK_PC3,/g
> s/[[:blank:]]*.pin =[[:blank:]]*20,\n/ RK_PC4,/g
> s/[[:blank:]]*.pin =[[:blank:]]*21,\n/ RK_PC5,/g
> s/[[:blank:]]*.pin =[[:blank:]]*22,\n/ RK_PC6,/g
> s/[[:blank:]]*.pin =[[:blank:]]*23,\n/ RK_PC7,/g
> s/[[:blank:]]*.pin =[[:blank:]]*24,\n/ RK_PD0,/g
> s/[[:blank:]]*.pin =[[:blank:]]*25,\n/ RK_PD1,/g
> s/[[:blank:]]*.pin =[[:blank:]]*26,\n/ RK_PD2,/g
> s/[[:blank:]]*.pin =[[:blank:]]*27,\n/ RK_PD3,/g
> s/[[:blank:]]*.pin =[[:blank:]]*28,\n/ RK_PD4,/g
> s/[[:blank:]]*.pin =[[:blank:]]*29,\n/ RK_PD5,/g
> s/[[:blank:]]*.pin =[[:blank:]]*30,\n/ RK_PD6,/g
> s/[[:blank:]]*.pin =[[:blank:]]*31,\n/ RK_PD7,/g
> s/[[:blank:]]*.func = \([[:digit:]]*,\)\n/ \1/g
> s/[[:blank:]]*.route_location =[[:blank:]]*\([[:print:]]*,\)\n//g
> s/[[:blank:]]*.route_offset = \(0x[[:xdigit:]]*,\)\n/ \1/g
> s/[[:blank:]]*.route_val =[[:blank:]]*\([[:print:]]*\),\n/ \1),/g
> s/\t{\n//g
> s/\t}, {\n//g
> s/\t},//g
> s/[[:blank:]]*\(\/\*[[:print:]]*\*\/\)\n[[:blank:]]*RK_MUXROUTE_SAME(\([[:print:]]*\)),\n/\tRK_MUXROUTE_SAME(\2),
> \1\n/g
> s/[[:blank:]]*\(\/\*[[:print:]]*\*\/\)\n[[:blank:]]*RK_MUXROUTE_SAME(\([[:print:]]*\)),/\tRK_MUXROUTE_SAME(\2),
> \1\n/g
> " drivers/pinctrl/pinctrl-rockchip.c
>
> Signed-off-by: Jianqun Xu
> Change-Id: Ifc823d9557605b9dfcc9c0455a739f04f3fce5be
Change-id should not be in here.
Somehow I remember giving this a "reviewed-by" before in some
previous version, but my memory may be faulty, so
Reviewed-by: Heiko Stuebner
and of course I like this change very much ;-)
Though it may be really helpful to not send these individual patches but
instead make it part of the series and put it at the beginning.
Tracking the order patches should get applied when they're sent individually
can get very hard.
Heiko
> ---
> drivers/pinctrl/pinctrl-rockchip.c | 669 +
> 1 file changed, 99 insertions(+), 570 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-rockchip.c
> b/drivers/pinctrl/pinctrl-rockchip.c
> index deabfbc74a01..6ba31c66ef8b 100644
> --- a/drivers/pinctrl/pinctrl-rockchip.c
> +++ b/drivers/pinctrl/pinctrl-rockchip.c
> @@ -292,6 +292,25 @@ struct rockchip_pin_bank {
> .pull_type[3] = pull3, \
> }
>
> +#define PIN_BANK_MUX_ROUTE_FLAGS(ID, PIN, FUNC, REG, VAL, FLAG)
> \
> + { \
> + .bank_num = ID, \
> + .pin= PIN, \
> + .func = FUNC, \
> + .route_offset = REG, \
> + .route_val = VAL, \
> + .route_location = FLAG, \
> + }
> +
> +#define RK_MUXROUTE_SAME(ID, PIN, FUNC, REG, VAL)\
> + PIN_BANK_MUX_ROUTE_FLAGS(ID, PIN, FUNC, REG, VAL, ROCKCHIP_ROUTE_SAME)
> +
> +#define RK_MUXROUTE_GRF(ID, PIN, FUNC, REG, VAL) \
> + PIN_BANK_MUX_ROUTE_FLAGS(ID, PIN, FUNC, REG, VAL, ROCKCHIP_ROUTE_GRF)
> +
> +#define RK_MUXROUTE_PMU(ID, PIN, FUNC, REG, VAL) \
> + PIN_BANK_MUX_ROUTE_FLAGS(ID, PIN, FUNC, REG, VAL, ROCKCHIP_ROUTE_PMU)
> +
> /**
> * struct rockchip_mux_recalced_data: represent a pin iomux data.
> * @num: bank number.
> @@ -803,597 +822,107 @@ static
Re: [PATCH v3 1/4] dt-bindings: pwm: convert pwm-rockchip.txt to YAML
Hi Thierry, Am Dienstag, 13. April 2021, 17:21:49 CEST schrieb Thierry Reding: > On Mon, Apr 12, 2021 at 10:01:52PM +0200, Johan Jonker wrote: > > Current dts files with 'pwm' nodes are manually verified. > > In order to automate this process pwm-rockchip.txt > > has to be converted to yaml. > > > > Signed-off-by: Johan Jonker > > --- > > For some SoC nodes this patch serie generates notifications > > for undocumented "interrupts" properties shared between > > PWM channels till there is consensus of what to do with it or > > someone makes a solution for the whole PWM block. > > > > Changed V3: > > fix mistake with compatibles introduced in V2 > > Changed V2: > > changed schema for clocks and clock-names > > --- > > .../devicetree/bindings/pwm/pwm-rockchip.txt | 27 --- > > .../devicetree/bindings/pwm/pwm-rockchip.yaml | 88 > > ++ > > 2 files changed, 88 insertions(+), 27 deletions(-) > > delete mode 100644 Documentation/devicetree/bindings/pwm/pwm-rockchip.txt > > create mode 100644 Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml > > Heiko, do you want to pick up patches 1 & 2 into your tree along with 3 & 4? > If so: > > Acked-by: Thierry Reding however you like :-) I can pick up everything for 5.14 for sure. Depending on your tree-schedule for the merge-window, you could also pick up all 4 with my: Acked-by: Heiko Stuebner Heiko
Re: [PATCH v2 0/7] gpio-rockchip driver
Am Montag, 12. April 2021, 14:13:37 CEST schrieb Andy Shevchenko: > On Sun, Apr 11, 2021 at 4:35 PM Peter Geis wrote: > > > > Separate gpio driver from pinctrl driver, and support v2 controller. > > > > Tested on rk3566-quartz64 prototype board. > > Can you give a bit more context? > Usually separation means that hardware is represented by two different > IP blocks that are (almost) independent to each other. Was it the case > on the original platforms? Do you have different pin controller (or > it's absent completely) on some new / old platform? They are separate on all Rockchip SoCs. I.e. the pinconfig (muxing, pulls, etc) is done via some registers inside the "General Register Files" [area for misc registers] and control for the gpio functionality is done in separate blocks for each bank. Lumping that stuff together into one driver, was a design-mistake from younger-me back in 2013 ;-) Heiko > > > > Patch History: > > V2 - Rebase to latest linux-next. > > > > Tested-by: Peter Geis > > > > Jianqun Xu (7): > > pinctrl/rockchip: separate struct rockchip_pin_bank to a head file > > pinctrl/pinctrl-rockchip.h: add pinctrl device to gpio bank struct > > gpio: separate gpio driver from pinctrl-rockchip driver > > gpio/rockchip: use struct rockchip_gpio_regs for gpio controller > > gpio/rockchip: support next version gpio controller > > gpio/rockchip: always enable clock for gpio controller > > gpio/rockchip: drop irq_gc_lock/irq_gc_unlock for irq set type > > > > drivers/gpio/Kconfig | 8 + > > drivers/gpio/Makefile | 1 + > > drivers/gpio/gpio-rockchip.c | 758 > > drivers/pinctrl/pinctrl-rockchip.c | 911 + > > drivers/pinctrl/pinctrl-rockchip.h | 287 + > > 5 files changed, 1073 insertions(+), 892 deletions(-) > > create mode 100644 drivers/gpio/gpio-rockchip.c > > create mode 100644 drivers/pinctrl/pinctrl-rockchip.h > > > > -- > > 2.25.1 > > > > >
Re: [PATCH v2 3/7] gpio: separate gpio driver from pinctrl-rockchip driver
Hi Johan, Am Sonntag, 11. April 2021, 19:51:52 CEST schrieb Johan Jonker: > Hi, > > When I check "rockchip,gpio-bank" with YAML it turns out that > rk3288-veyron-XXX has 'gpio-line-names' as 'extra' property. > It is not defined in the "rockchip,pinctrl.txt" document, but in > ~/.local/lib/python3.5/site-packages/dtschema/schemas/gpio/gpio.yaml > > Where is that in use? > In this driver or external? > Can it be removed from mainline dts? gpio-line-names gets defined in devicetree/bindings/gpio/gpio.txt which isn't converted to yaml yet: Optionally, a GPIO controller may have a "gpio-line-names" property. This is an array of strings defining the names of the GPIO lines going out of the GPIO controller. So that property is perfectly fine where it is. Heiko > > Johan > > /arch/arm/boot/dts/rk3288-veyron-fievel.dt.yaml: gpio7@ff7e: > 'gpio-line-names' does not match any of the regexes: 'pinctrl-[0-9]+' > From schema: > /Documentation/devicetree/bindings/gpio/rockchip,gpio-bank.yaml > > On 4/11/21 3:30 PM, Peter Geis wrote: > > From: Jianqun Xu > > > > Separate the gpio driver from the pinctrl driver. > > > > Signed-off-by: Jianqun Xu > > --- >
Re: [PATCH] dt-bindings: pinctrl: rockchip: add RK3568 SoC support
Am Samstag, 10. April 2021, 22:45:00 CEST schrieb Ezequiel Garcia: > Add RK3568/RK3566 SoC support to pinctrl. > > Signed-off-by: Ezequiel Garcia Reviewed-by: Heiko Stuebner > --- > Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.txt | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.txt > b/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.txt > index d3eae61a340d..91fab614c381 100644 > --- a/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.txt > +++ b/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.txt > @@ -33,6 +33,7 @@ Required properties for iomux controller: > "rockchip,rk3328-pinctrl": for Rockchip RK3328 > "rockchip,rk3368-pinctrl": for Rockchip RK3368 > "rockchip,rk3399-pinctrl": for Rockchip RK3399 > + "rockchip,rk3568-pinctrl": for Rockchip RK3568 > >- rockchip,grf: phandle referencing a syscon providing the >"general register files" >
Re: [RESEND PATCH v5 07/11] soc: rockchip: pm-domains: Add a meaningful power domain name
Hi Elaine again,
Am Freitag, 26. März 2021, 10:52:53 CET schrieb Heiko Stübner:
> Hi Elaine,
>
> [also adding that answer to the resend :-) ]
>
> Am Freitag, 26. März 2021, 10:17:39 CET schrieb Elaine Zhang:
> > Add the power domains names to the power domain info struct so we
> > have meaningful name for every power domain.
> >
> > Signed-off-by: Elaine Zhang
>
> I like that approach very much, there is one tiny comment below.
>
> > ---
> > drivers/soc/rockchip/pm_domains.c | 217 +++---
> > 1 file changed, 112 insertions(+), 105 deletions(-)
> >
> > diff --git a/drivers/soc/rockchip/pm_domains.c
> > b/drivers/soc/rockchip/pm_domains.c
> > index 54eb6cfc5d5b..d661d967079f 100644
> > --- a/drivers/soc/rockchip/pm_domains.c
> > +++ b/drivers/soc/rockchip/pm_domains.c
> > @@ -29,6 +29,7 @@
> > #include
> >
> > struct rockchip_domain_info {
> > + const char *name;
> > int pwr_mask;
> > int status_mask;
> > int req_mask;
> > @@ -85,8 +86,9 @@ struct rockchip_pmu {
> >
> > #define to_rockchip_pd(gpd) container_of(gpd, struct rockchip_pm_domain,
> > genpd)
> >
> > -#define DOMAIN(pwr, status, req, idle, ack, wakeup)\
> > +#define DOMAIN(_name, pwr, status, req, idle, ack, wakeup) \
> > { \
> > + .name = _name, \
> > .pwr_mask = (pwr), \
> > .status_mask = (status),\
> > .req_mask = (req), \
> > @@ -95,8 +97,9 @@ struct rockchip_pmu {
> > .active_wakeup = (wakeup), \
> > }
> >
> > -#define DOMAIN_M(pwr, status, req, idle, ack, wakeup) \
> > +#define DOMAIN_M(_name, pwr, status, req, idle, ack, wakeup) \
> > { \
> > + .name = _name, \
> > .pwr_w_mask = (pwr) << 16, \
> > .pwr_mask = (pwr), \
> > .status_mask = (status),\
> > @@ -107,8 +110,9 @@ struct rockchip_pmu {
> > .active_wakeup = wakeup,\
> > }
> >
> > -#define DOMAIN_RK3036(req, ack, idle, wakeup) \
> > +#define DOMAIN_RK3036(_name, req, ack, idle, wakeup) \
> > { \
> > + .name = _name, \
> > .req_mask = (req), \
> > .req_w_mask = (req) << 16, \
> > .ack_mask = (ack), \
> > @@ -119,17 +123,17 @@ struct rockchip_pmu {
> > #define DOMAIN_PX30(pwr, status, req, wakeup) \
> > DOMAIN_M(pwr, status, req, (req) << 16, req, wakeup)
> >
> > -#define DOMAIN_RK3288(pwr, status, req, wakeup)\
> > - DOMAIN(pwr, status, req, req, (req) << 16, wakeup)
> > +#define DOMAIN_RK3288(name, pwr, status, req, wakeup) \
> > + DOMAIN(name, pwr, status, req, req, (req) << 16, wakeup)
> >
> > -#define DOMAIN_RK3328(pwr, status, req, wakeup)\
> > - DOMAIN_M(pwr, pwr, req, (req) << 10, req, wakeup)
> > +#define DOMAIN_RK3328(name, pwr, status, req, wakeup) \
> > + DOMAIN_M(name, pwr, pwr, req, (req) << 10, req, wakeup)
> >
> > -#define DOMAIN_RK3368(pwr, status, req, wakeup)\
> > - DOMAIN(pwr, status, req, (req) << 16, req, wakeup)
> > +#define DOMAIN_RK3368(name, pwr, status, req, wakeup) \
> > + DOMAIN(name, pwr, status, req, (req) << 16, req, wakeup)
> >
> > -#define DOMAIN_RK3399(pwr, status, req, wakeup)\
> > - DOMAIN(pwr, status, req, req, req, wakeup)
> > +#define DOMAIN_RK3399(name, pwr, status, req, wakeup) \
> > + DOMAIN(name, pwr, status, req, req, req, wakeup)
> >
> > static bool rockchip_pmu_domain_is_idle(struct rockchip_pm_domain *pd)
> > {
> > @@ -490,7 +494,10 @@ static int rockchip_pm_add_one_domain(struct
> > rockchip_pmu *pmu,
> > goto err_unprepare_clocks;
> > }
> >
> > - pd->genpd.name = node->name;
> > + if (!pd->info->name)
> > + pd->genpd.name = node->name;
>
> I guess we should make that "node->full_name" perhaps?
> And I guess it might be nicer to swap the cases
>
> if (pd->info->name)
> pd->genpd.name = pd->i
Re: [RESEND PATCH v5 07/11] soc: rockchip: pm-domains: Add a meaningful power domain name
Hi Elaine,
[also adding that answer to the resend :-) ]
Am Freitag, 26. März 2021, 10:17:39 CET schrieb Elaine Zhang:
> Add the power domains names to the power domain info struct so we
> have meaningful name for every power domain.
>
> Signed-off-by: Elaine Zhang
I like that approach very much, there is one tiny comment below.
> ---
> drivers/soc/rockchip/pm_domains.c | 217 +++---
> 1 file changed, 112 insertions(+), 105 deletions(-)
>
> diff --git a/drivers/soc/rockchip/pm_domains.c
> b/drivers/soc/rockchip/pm_domains.c
> index 54eb6cfc5d5b..d661d967079f 100644
> --- a/drivers/soc/rockchip/pm_domains.c
> +++ b/drivers/soc/rockchip/pm_domains.c
> @@ -29,6 +29,7 @@
> #include
>
> struct rockchip_domain_info {
> + const char *name;
> int pwr_mask;
> int status_mask;
> int req_mask;
> @@ -85,8 +86,9 @@ struct rockchip_pmu {
>
> #define to_rockchip_pd(gpd) container_of(gpd, struct rockchip_pm_domain,
> genpd)
>
> -#define DOMAIN(pwr, status, req, idle, ack, wakeup)\
> +#define DOMAIN(_name, pwr, status, req, idle, ack, wakeup) \
> { \
> + .name = _name, \
> .pwr_mask = (pwr), \
> .status_mask = (status),\
> .req_mask = (req), \
> @@ -95,8 +97,9 @@ struct rockchip_pmu {
> .active_wakeup = (wakeup), \
> }
>
> -#define DOMAIN_M(pwr, status, req, idle, ack, wakeup) \
> +#define DOMAIN_M(_name, pwr, status, req, idle, ack, wakeup) \
> { \
> + .name = _name, \
> .pwr_w_mask = (pwr) << 16, \
> .pwr_mask = (pwr), \
> .status_mask = (status),\
> @@ -107,8 +110,9 @@ struct rockchip_pmu {
> .active_wakeup = wakeup,\
> }
>
> -#define DOMAIN_RK3036(req, ack, idle, wakeup) \
> +#define DOMAIN_RK3036(_name, req, ack, idle, wakeup) \
> { \
> + .name = _name, \
> .req_mask = (req), \
> .req_w_mask = (req) << 16, \
> .ack_mask = (ack), \
> @@ -119,17 +123,17 @@ struct rockchip_pmu {
> #define DOMAIN_PX30(pwr, status, req, wakeup) \
> DOMAIN_M(pwr, status, req, (req) << 16, req, wakeup)
>
> -#define DOMAIN_RK3288(pwr, status, req, wakeup)\
> - DOMAIN(pwr, status, req, req, (req) << 16, wakeup)
> +#define DOMAIN_RK3288(name, pwr, status, req, wakeup) \
> + DOMAIN(name, pwr, status, req, req, (req) << 16, wakeup)
>
> -#define DOMAIN_RK3328(pwr, status, req, wakeup)\
> - DOMAIN_M(pwr, pwr, req, (req) << 10, req, wakeup)
> +#define DOMAIN_RK3328(name, pwr, status, req, wakeup) \
> + DOMAIN_M(name, pwr, pwr, req, (req) << 10, req, wakeup)
>
> -#define DOMAIN_RK3368(pwr, status, req, wakeup)\
> - DOMAIN(pwr, status, req, (req) << 16, req, wakeup)
> +#define DOMAIN_RK3368(name, pwr, status, req, wakeup) \
> + DOMAIN(name, pwr, status, req, (req) << 16, req, wakeup)
>
> -#define DOMAIN_RK3399(pwr, status, req, wakeup)\
> - DOMAIN(pwr, status, req, req, req, wakeup)
> +#define DOMAIN_RK3399(name, pwr, status, req, wakeup) \
> + DOMAIN(name, pwr, status, req, req, req, wakeup)
>
> static bool rockchip_pmu_domain_is_idle(struct rockchip_pm_domain *pd)
> {
> @@ -490,7 +494,10 @@ static int rockchip_pm_add_one_domain(struct
> rockchip_pmu *pmu,
> goto err_unprepare_clocks;
> }
>
> - pd->genpd.name = node->name;
> + if (!pd->info->name)
> + pd->genpd.name = node->name;
I guess we should make that "node->full_name" perhaps?
And I guess it might be nicer to swap the cases
if (pd->info->name)
pd->genpd.name = pd->info->name
else
pd->genpd.name = node->full_name;
for easier readability
Heiko
> + else
> + pd->genpd.name = pd->info->name;
> pd->genpd.power_off = rockchip_pd_power_off;
> pd->genpd.power_on = rockchip_pd_power_on;
> pd->genpd.attach_dev = rockchip_pd_attach_dev;
> @@ -716,129 +723,129 @@ static int rockchip_pm_domain_probe(struct
> platform_device *pdev)
> }
>
> static const struct rockchip_domain_info px30_pm_domains[] = {
> - [PX30_PD_USB] = DOMAIN_PX30(BIT(5), BIT(5), BIT(10),
> false),
> - [PX30_PD_SDCARD]= DOMAIN_PX30(BIT(8), BIT(8), BIT(9),
> false),
> - [PX30_PD_GMAC] = DOMAIN_PX30(BIT(10), BIT(10), BIT(6),
> false),
> - [PX30_PD_MMC_NAND] = DOMAIN_PX30(BIT(11), BIT(11), BIT(5),
> false),
> - [PX30_PD_VPU] = DOMAIN_PX30(BIT(12), BIT(12), BIT(14),
> false),
> - [PX30_PD_VO]= DOMAIN_PX30(BIT(13), BIT(13),
Re: [PATCH v5 09/11] dt-bindings: power: rockchip: Convert to json-schema
Am Freitag, 26. März 2021, 10:18:03 CET schrieb Elaine Zhang: > Convert the soc/rockchip/power_domain.txt binding document to > json-schema and move to the power bindings directory. > > Signed-off-by: Enric Balletbo i Serra > Signed-off-by: Elaine Zhang If Enric is the original author of this patch, maybe this needs a matching "From: ... " line? Thanks Heiko > --- > .../power/rockchip,power-controller.yaml | 291 ++ > .../bindings/soc/rockchip/power_domain.txt| 136 > 2 files changed, 291 insertions(+), 136 deletions(-) > create mode 100644 > Documentation/devicetree/bindings/power/rockchip,power-controller.yaml > delete mode 100644 > Documentation/devicetree/bindings/soc/rockchip/power_domain.txt > > diff --git > a/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml > b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml > new file mode 100644 > index ..9fec9e227432 > --- /dev/null > +++ b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml > @@ -0,0 +1,291 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/power/rockchip,power-controller.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Rockchip Power Domains > + > +maintainers: > + - Elaine Zhang > + - Heiko Stuebner > + > +description: | > + Rockchip processors include support for multiple power domains which can be > + powered up/down by software based on different application scenarios to > save power. > + > + Power domains contained within power-controller node are generic power > domain > + providers documented in > Documentation/devicetree/bindings/power/power-domain.yaml. > + > + IP cores belonging to a power domain should contain a "power-domains" > + property that is a phandle for the power domain node representing the > domain. > + > +properties: > + $nodename: > +const: power-controller > + > + compatible: > +enum: > + - rockchip,px30-power-controller > + - rockchip,rk3036-power-controller > + - rockchip,rk3066-power-controller > + - rockchip,rk3128-power-controller > + - rockchip,rk3188-power-controller > + - rockchip,rk3228-power-controller > + - rockchip,rk3288-power-controller > + - rockchip,rk3328-power-controller > + - rockchip,rk3366-power-controller > + - rockchip,rk3368-power-controller > + - rockchip,rk3399-power-controller > + > + "#power-domain-cells": > +const: 1 > + > + "#address-cells": > +const: 1 > + > + "#size-cells": > +const: 0 > + > + clocks: true > + > + assigned-clocks: > +minItems: 1 > + > + assigned-clock-parents: > +minItems: 1 > + > +patternProperties: > + "^power-domain@[0-9a-f]+$": > +type: object > +description: | > + Represents the power domains within the power controller node as > documented > + in Documentation/devicetree/bindings/power/power-domain.yaml. > + > +properties: > + > + "#power-domain-cells": > +description: > + Must be 0 for nodes representing a single PM domain and 1 for nodes > + providing multiple PM domains. > + > + "#address-cells": > +const: 1 > + > + "#size-cells": > +const: 0 > + > + reg: > +maxItems: 1 > +description: | > + Power domain index. Valid values are defined in > + "include/dt-bindings/power/px30-power.h" > + "include/dt-bindings/power/rk3036-power.h" > + "include/dt-bindings/power/rk3066-power.h" > + "include/dt-bindings/power/rk3128-power.h" > + "include/dt-bindings/power/rk3188-power.h" > + "include/dt-bindings/power/rk3228-power.h" > + "include/dt-bindings/power/rk3288-power.h" > + "include/dt-bindings/power/rk3328-power.h" > + "include/dt-bindings/power/rk3366-power.h" > + "include/dt-bindings/power/rk3368-power.h" > + "include/dt-bindings/power/rk3399-power.h" > + > + clocks: > +description: | > + A number of phandles to clocks that need to be enabled while power > domain > + switches state. > + > + pm_qos: > +description: | > + A number of phandles to qos blocks which need to be saved and > restored > + while power domain switches state. > + > +patternProperties: > + "^power-domain@[0-9a-f]+$": > +type: object > +description: | > + Represents a power domain child within a power domain parent node. > + > +properties: > + > + "#power-domain-cells": > +description: > + Must be 0 for nodes representing a single PM domain and 1 for > nodes > + providing multiple PM domains. > + > + "#address-cells": > +const: 1 > + > + "#size-cells": > +const: 0 > + > +
Re: [PATCH v5 07/11] soc: rockchip: pm-domains: Add a meaningful power domain name
Hi Elaine,
Am Freitag, 26. März 2021, 10:17:39 CET schrieb Elaine Zhang:
> Add the power domains names to the power domain info struct so we
> have meaningful name for every power domain.
>
> Signed-off-by: Elaine Zhang
I like that approach very much, there is one tiny comment below.
> ---
> drivers/soc/rockchip/pm_domains.c | 217 +++---
> 1 file changed, 112 insertions(+), 105 deletions(-)
>
> diff --git a/drivers/soc/rockchip/pm_domains.c
> b/drivers/soc/rockchip/pm_domains.c
> index 54eb6cfc5d5b..d661d967079f 100644
> --- a/drivers/soc/rockchip/pm_domains.c
> +++ b/drivers/soc/rockchip/pm_domains.c
> @@ -29,6 +29,7 @@
> #include
>
> struct rockchip_domain_info {
> + const char *name;
> int pwr_mask;
> int status_mask;
> int req_mask;
> @@ -85,8 +86,9 @@ struct rockchip_pmu {
>
> #define to_rockchip_pd(gpd) container_of(gpd, struct rockchip_pm_domain,
> genpd)
>
> -#define DOMAIN(pwr, status, req, idle, ack, wakeup) \
> +#define DOMAIN(_name, pwr, status, req, idle, ack, wakeup) \
> {\
> + .name = _name, \
> .pwr_mask = (pwr), \
> .status_mask = (status),\
> .req_mask = (req), \
> @@ -95,8 +97,9 @@ struct rockchip_pmu {
> .active_wakeup = (wakeup), \
> }
>
> -#define DOMAIN_M(pwr, status, req, idle, ack, wakeup)\
> +#define DOMAIN_M(_name, pwr, status, req, idle, ack, wakeup) \
> {\
> + .name = _name, \
> .pwr_w_mask = (pwr) << 16, \
> .pwr_mask = (pwr), \
> .status_mask = (status),\
> @@ -107,8 +110,9 @@ struct rockchip_pmu {
> .active_wakeup = wakeup,\
> }
>
> -#define DOMAIN_RK3036(req, ack, idle, wakeup)\
> +#define DOMAIN_RK3036(_name, req, ack, idle, wakeup) \
> {\
> + .name = _name, \
> .req_mask = (req), \
> .req_w_mask = (req) << 16, \
> .ack_mask = (ack), \
> @@ -119,17 +123,17 @@ struct rockchip_pmu {
> #define DOMAIN_PX30(pwr, status, req, wakeup)\
> DOMAIN_M(pwr, status, req, (req) << 16, req, wakeup)
>
> -#define DOMAIN_RK3288(pwr, status, req, wakeup) \
> - DOMAIN(pwr, status, req, req, (req) << 16, wakeup)
> +#define DOMAIN_RK3288(name, pwr, status, req, wakeup)\
> + DOMAIN(name, pwr, status, req, req, (req) << 16, wakeup)
>
> -#define DOMAIN_RK3328(pwr, status, req, wakeup) \
> - DOMAIN_M(pwr, pwr, req, (req) << 10, req, wakeup)
> +#define DOMAIN_RK3328(name, pwr, status, req, wakeup)\
> + DOMAIN_M(name, pwr, pwr, req, (req) << 10, req, wakeup)
>
> -#define DOMAIN_RK3368(pwr, status, req, wakeup) \
> - DOMAIN(pwr, status, req, (req) << 16, req, wakeup)
> +#define DOMAIN_RK3368(name, pwr, status, req, wakeup)\
> + DOMAIN(name, pwr, status, req, (req) << 16, req, wakeup)
>
> -#define DOMAIN_RK3399(pwr, status, req, wakeup) \
> - DOMAIN(pwr, status, req, req, req, wakeup)
> +#define DOMAIN_RK3399(name, pwr, status, req, wakeup)\
> + DOMAIN(name, pwr, status, req, req, req, wakeup)
>
> static bool rockchip_pmu_domain_is_idle(struct rockchip_pm_domain *pd)
> {
> @@ -490,7 +494,10 @@ static int rockchip_pm_add_one_domain(struct
> rockchip_pmu *pmu,
> goto err_unprepare_clocks;
> }
>
> - pd->genpd.name = node->name;
> + if (!pd->info->name)
> + pd->genpd.name = node->name;
I guess we should make that "node->full_name" perhaps?
And I guess it might be nicer to swap the cases
if (pd->info->name)
pd->genpd.name = pd->info->name
else
pd->genpd.name = node->full_name;
for easier readability
Heiko
> + else
> + pd->genpd.name = pd->info->name;
> pd->genpd.power_off = rockchip_pd_power_off;
> pd->genpd.power_on = rockchip_pd_power_on;
> pd->genpd.attach_dev = rockchip_pd_attach_dev;
> @@ -716,129 +723,129 @@ static int rockchip_pm_domain_probe(struct
> platform_device *pdev)
> }
>
> static const struct rockchip_domain_info px30_pm_domains[] = {
> - [PX30_PD_USB] = DOMAIN_PX30(BIT(5), BIT(5), BIT(10), false),
> - [PX30_PD_SDCARD]= DOMAIN_PX30(BIT(8), BIT(8), BIT(9), false),
> - [PX30_PD_GMAC] = DOMAIN_PX30(BIT(10), BIT(10), BIT(6), false),
> - [PX30_PD_MMC_NAND] = DOMAIN_PX30(BIT(11), BIT(11), BIT(5), false),
> - [PX30_PD_VPU] = DOMAIN_PX30(BIT(12), BIT(12), BIT(14),
Re: [PATCH v5 2/8] usb: dwc3: of-simple: bail probe if no dwc3 child node
Hi Greg, Felipe, Am Dienstag, 9. Februar 2021, 20:23:44 CET schrieb Johan Jonker: > For some of the dwc3-of-simple compatible SoCs we > don't want to bind this driver to a dwc3 node, > but bind that node to the 'snps,dwc3' driver instead. > The kernel has no logic to decide which driver to bind > to if there are 2 matching drivers, so bail probe if no > dwc3 child node. > > Signed-off-by: Johan Jonker It looks like this patch fell through the cracks? I.e. I can see patches 1+6 adding the devicetree bindings in 5.12-rc but haven't found this patch there. And looking at https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/log/drivers/usb/dwc3/dwc3-of-simple.c?h=usb-next I also didn't find it. >From what I gathered that was somehow mandated from the Devicetree side as the dwc3-subnode system merely is some Linux-specific thingy and should not be enforced if not explicitly needed. I guess Johan can provide pointers to the previous discussion. So could you look at applying this patch to some usb-tree? Thanks Heiko > --- > drivers/usb/dwc3/dwc3-of-simple.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/usb/dwc3/dwc3-of-simple.c > b/drivers/usb/dwc3/dwc3-of-simple.c > index e62ecd22b..347b4d384 100644 > --- a/drivers/usb/dwc3/dwc3-of-simple.c > +++ b/drivers/usb/dwc3/dwc3-of-simple.c > @@ -38,6 +38,10 @@ static int dwc3_of_simple_probe(struct platform_device > *pdev) > > int ret; > > + /* Bail probe if no dwc3 child node. */ > + if (!of_get_compatible_child(dev->of_node, "snps,dwc3")) > + return -ENODEV; > + > simple = devm_kzalloc(dev, sizeof(*simple), GFP_KERNEL); > if (!simple) > return -ENOMEM; >
Re: [PATCH 3/6] drm/rockchip: dsi: add ability to work as a phy instead of full dsi
Am Montag, 15. Februar 2021, 15:33:19 CET schrieb Helen Koike:
> > From: Heiko Stuebner
> > diff --git a/drivers/gpu/drm/rockchip/Kconfig
> > b/drivers/gpu/drm/rockchip/Kconfig
> > index cb25c0e8fc9b..3094d4533ad6 100644
> > --- a/drivers/gpu/drm/rockchip/Kconfig
> > +++ b/drivers/gpu/drm/rockchip/Kconfig
> > @@ -9,6 +9,8 @@ config DRM_ROCKCHIP
> > select DRM_ANALOGIX_DP if ROCKCHIP_ANALOGIX_DP
> > select DRM_DW_HDMI if ROCKCHIP_DW_HDMI
> > select DRM_DW_MIPI_DSI if ROCKCHIP_DW_MIPI_DSI
> > + select GENERIC_PHY if ROCKCHIP_DW_MIPI_DSI
> > + select GENERIC_PHY_MIPI_DPHY if ROCKCHIP_DW_MIPI_DSI
>
> maybe alphabetical order?
ok
> > +static int dw_mipi_dsi_dphy_power_on(struct phy *phy)
> > +{
> > + struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> > + int i, ret;
>
> It seems "i" could be removed, use ret instead.
I don't think so
I.e. the driver does
i = max_mbps_to_parameter(...)
...
ret = power-on-clocks-and-stuff
...
dw_mipi_dsi_phy_write( dppa_map[i].hsfreqrange)
So will need to keep the param index separate.
> In general, the patch doesn't look wrong to me.
>
> For the whole serie:
> Acked-by: Helen Koike
Thanks a lot :-)
Heiko
Re: [PATCH v4 2/4] dt-bindings: power: rockchip: Convert to json-schema
Am Mittwoch, 24. März 2021, 11:32:42 CET schrieb Enric Balletbo i Serra:
>
> On 24/3/21 11:25, Enric Balletbo i Serra wrote:
> > Hi Elaine,
> >
> > On 24/3/21 11:18, elaine.zhang wrote:
> >> Hi, Enric
> >>
> >> 在 2021/3/24 下午5:56, Enric Balletbo i Serra 写道:
> >>> Hi Elaine,
> >>>
> >>> This is not the exact version I sent, and you reintroduced a "problem"
> >>> that were
> >>> already solved/discussed on previous versions. See below:
> >>>
> >>> On 24/3/21 8:16, Elaine Zhang wrote:
> Convert the soc/rockchip/power_domain.txt binding document to
> json-schema and move to the power bindings directory.
>
> Signed-off-by: Enric Balletbo i Serra
> >>> If you do significant is a good practice shortly describe them within []
> >>> here.
> >>>
> Signed-off-by: Elaine Zhang
> >>> Note that my last version already had the
> >>>
> >>> Reviewed-by: Rob Herring
> >>>
> >>> Which should be fine for merging (with probably only minor changes) and
> >>> you
> >>> could maintain if you don't do significant changes, but that's not the
> >>> case, as
> >>> I said, you are reintroducing one problem. Please review the comments
> >>> already
> >>> received on this patchset or similar patchsets to avoid this.
> >>>
> ---
> .../power/rockchip,power-controller.yaml | 284 ++
> .../bindings/soc/rockchip/power_domain.txt| 136 -
> 2 files changed, 284 insertions(+), 136 deletions(-)
> create mode 100644
> Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
> delete mode 100644
> Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
>
> diff --git
> a/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
> b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
> new file mode 100644
> index ..a220322c5139
> --- /dev/null
> +++
> b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
> @@ -0,0 +1,284 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/rockchip,power-controller.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Rockchip Power Domains
> +
> +maintainers:
> + - Elaine Zhang
> + - Rob Herring
> >>> Up to Rob, but I don't think Rob would like to be the maintainer. I think
> >>> you
> >>> can only include yourself and Heiko.
> >>>
> >>>
> + - Heiko Stuebner
> +
> +description: |
> + Rockchip processors include support for multiple power domains which
> can be
> + powered up/down by software based on different application scenarios
> to
> save power.
> +
> + Power domains contained within power-controller node are generic
> power domain
> + providers documented in
> Documentation/devicetree/bindings/power/power-domain.yaml.
> +
> + IP cores belonging to a power domain should contain a "power-domains"
> + property that is a phandle for the power domain node representing the
> domain.
> +
> +properties:
> + $nodename:
> +const: power-controller
> +
> + compatible:
> +enum:
> + - rockchip,px30-power-controller
> + - rockchip,rk3036-power-controller
> + - rockchip,rk3066-power-controller
> + - rockchip,rk3128-power-controller
> + - rockchip,rk3188-power-controller
> + - rockchip,rk3228-power-controller
> + - rockchip,rk3288-power-controller
> + - rockchip,rk3328-power-controller
> + - rockchip,rk3366-power-controller
> + - rockchip,rk3368-power-controller
> + - rockchip,rk3399-power-controller
> +
> + "#power-domain-cells":
> +const: 1
> +
> + "#address-cells":
> +const: 1
> +
> + "#size-cells":
> +const: 0
> +
> +patternProperties:
> + "^pd_[0-9a-z_]{2,10}@[0-9a-f]+$":
> +type: object
> +description: |
> + Represents the power domains within the power controller node as
> documented
> + in Documentation/devicetree/bindings/power/power-domain.yaml.
> +
> >>> The node names must be generic, as this is power-domain must be in the
> >>> form:
> >>>
> >>> +patternProperties:
> >>> + "^power-domain@[0-9a-f]+$":
> >> In this way, dtbs_check cannot be passed, and all the usage methods in dts
> >> of
> >> Rockchip need to be corrected, which I think is a bigger change.
> >
> > Well, the problem is in the Rockchip dtbs, so needs to be fixed there. The
> > bindings must describe hardware in a generic way, not describe the actual
> > dtbs
> > to not report errors.
> >
>
> FWIW I remember I did something regarding this but never sent to upstream,
> feel
> free to
Re: [PATCH v3 2/2] rockchip: rk3399: Add support for FriendlyARM NanoPi R4S
Am Montag, 15. März 2021, 17:38:37 CET schrieb Geert Uytterhoeven:
> Hi Robin,
>
> On Mon, Mar 15, 2021 at 5:32 PM Robin Murphy wrote:
> > On 2021-03-13 13:22, CN_SZTL wrote:
> > > Robin Murphy 于2021年3月13日周六 下午7:55写道：
> > >>
> > >> On 2021-03-13 03:25, Tianling Shen wrote:
> > >>> + gpio-leds {
> > >>> + compatible = "gpio-leds";
> > >>> + pinctrl-0 = <_led_pin>, <_led_pin>,
> > >>> <_led_pin>;
> > >>> + pinctrl-names = "default";
> > >>> +
> > >>> + lan_led: led-0 {
> > >>> + gpios = < RK_PA1 GPIO_ACTIVE_HIGH>;
> > >>> + label = "nanopi-r4s:green:lan";
> > >>> + };
> > >>> +
> > >>> + sys_led: led-1 {
> > >>> + gpios = < RK_PB5 GPIO_ACTIVE_HIGH>;
> > >>> + label = "nanopi-r4s:red:sys";
> > >>> + default-state = "on";
> > >>> + };
> > >>> +
> > >>> + wan_led: led-2 {
> > >>> + gpios = < RK_PA0 GPIO_ACTIVE_HIGH>;
> > >>> + label = "nanopi-r4s:green:wan";
> > >>> + };
> >
> > Nit: (apologies for overlooking it before) there isn't an obvious
> > definitive order for the LEDs, but the order here is certainly not
> > consistent with anything. The most logical would probably be sys, wan,
>
> Looks like alphabetical sort order to me ;-)
yep ... led-0, led-1, led-2 looks pretty sorted ;-)
Generally I'd prefer sorting by node-names ... especially as the phandle
is sort of optional for most things - and they sometimes come and go
in dt-files.
Heiko
Re: [PATCH] spi: rockchip: avoid objtool warning
Am Donnerstag, 25. Februar 2021, 13:55:34 CET schrieb Arnd Bergmann:
> From: Arnd Bergmann
>
> Building this file with clang leads to a an unreachable code path
> causing a warning from objtool:
>
> drivers/spi/spi-rockchip.o: warning: objtool:
> rockchip_spi_transfer_one()+0x2e0: sibling call from callable instruction
> with modified stack frame
>
> Use BUG() instead of unreachable() to avoid the undefined behavior
> if it does happen.
>
> Fixes: 65498c6ae241 ("spi: rockchip: support 4bit words")
> Signed-off-by: Arnd Bergmann
Acked-by: Heiko Stuebner
> ---
> drivers/spi/spi-rockchip.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
> index 936ef54e0903..972beac1169a 100644
> --- a/drivers/spi/spi-rockchip.c
> +++ b/drivers/spi/spi-rockchip.c
> @@ -521,7 +521,7 @@ static void rockchip_spi_config(struct rockchip_spi *rs,
>* ctlr->bits_per_word_mask, so this shouldn't
>* happen
>*/
> - unreachable();
> + BUG();
> }
>
> if (use_dma) {
>
Re: [PATCH v1 3/4] clk: rockchip: support more core div setting
Hi Elaine,
Am Donnerstag, 25. Februar 2021, 03:59:32 CET schrieb elaine.zhang:
> 在 2021/2/23 下午6:22, Heiko Stübner 写道:
> > Am Dienstag, 23. Februar 2021, 10:53:51 CET schrieb Elaine Zhang:
> >> A55 supports each core to work at different frequencies, and each core
> >> has an independent divider control.
> >>
> >> Signed-off-by: Elaine Zhang
> >> ---
> >> drivers/clk/rockchip/clk-cpu.c | 25 +
> >> drivers/clk/rockchip/clk.h | 17 -
> >> 2 files changed, 41 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/clk/rockchip/clk-cpu.c
> >> b/drivers/clk/rockchip/clk-cpu.c
> >> index fa9027fb1920..cac06f4f7573 100644
> >> --- a/drivers/clk/rockchip/clk-cpu.c
> >> +++ b/drivers/clk/rockchip/clk-cpu.c
> >> @@ -164,6 +164,18 @@ static int rockchip_cpuclk_pre_rate_change(struct
> >> rockchip_cpuclk *cpuclk,
> >> reg_data->mux_core_mask,
> >> reg_data->mux_core_shift),
> >> cpuclk->reg_base + reg_data->core_reg);
> >> + if (reg_data->core1_reg)
> >> + writel(HIWORD_UPDATE(alt_div, reg_data->div_core1_mask,
> >> + reg_data->div_core1_shift),
> >> + cpuclk->reg_base + reg_data->core1_reg);
> >> + if (reg_data->core2_reg)
> >> + writel(HIWORD_UPDATE(alt_div, reg_data->div_core2_mask,
> >> + reg_data->div_core2_shift),
> >> + cpuclk->reg_base + reg_data->core2_reg);
> >> + if (reg_data->core3_reg)
> >> + writel(HIWORD_UPDATE(alt_div, reg_data->div_core3_mask,
> >> + reg_data->div_core3_shift),
> >> + cpuclk->reg_base + reg_data->core3_reg);
> > for (i = 0; i < reg_data->num_cores; i++)
> > writel(...)
> >
> >>} else {
> >>/* select alternate parent */
> >>writel(HIWORD_UPDATE(reg_data->mux_core_alt,
> >> @@ -209,6 +221,19 @@ static int rockchip_cpuclk_post_rate_change(struct
> >> rockchip_cpuclk *cpuclk,
> >>reg_data->mux_core_shift),
> >> cpuclk->reg_base + reg_data->core_reg);
> >>
> >> + if (reg_data->core1_reg)
> >> + writel(HIWORD_UPDATE(0, reg_data->div_core1_mask,
> >> + reg_data->div_core1_shift),
> >> + cpuclk->reg_base + reg_data->core1_reg);
> >> + if (reg_data->core2_reg)
> >> + writel(HIWORD_UPDATE(0, reg_data->div_core2_mask,
> >> + reg_data->div_core2_shift),
> >> + cpuclk->reg_base + reg_data->core2_reg);
> >> + if (reg_data->core3_reg)
> >> + writel(HIWORD_UPDATE(0, reg_data->div_core3_mask,
> >> + reg_data->div_core3_shift),
> >> + cpuclk->reg_base + reg_data->core3_reg);
> >> +
> > for (i = 0; i < reg_data->num_cores; i++)
> > writel(...)
> >
> >>if (ndata->old_rate > ndata->new_rate)
> >>rockchip_cpuclk_set_dividers(cpuclk, rate);
> >>
> >> diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h
> >> index 2271a84124b0..b46c93fd0cb5 100644
> >> --- a/drivers/clk/rockchip/clk.h
> >> +++ b/drivers/clk/rockchip/clk.h
> >> @@ -322,7 +322,7 @@ struct rockchip_cpuclk_clksel {
> >>u32 val;
> >> };
> >>
> >> -#define ROCKCHIP_CPUCLK_NUM_DIVIDERS 2
> >> +#define ROCKCHIP_CPUCLK_NUM_DIVIDERS 5
> > please move this into a separate patch, as yes the rk3568 needs more
> > dividers but that isn't related to adding separate core divider controls.
> >
> > [...]
> > add
> >
> > #define ROCKCHIP_CPUCLK_MAX_CORES 4
> >
> >> struct rockchip_cpuclk_rate_table {
> >>unsigned long prate;
> >>struct rockchip_cpuclk_clksel divs[ROCKCHIP_CPUCLK_NUM_DIVIDERS];
> >> @@ -333,6 +333,12 @@ struct rockchip_cpuclk_rate_table {
> >>* @core_reg:register offset of the core settings register
> >>* @div_core_shift: core divider offset used to divide th
Re: [PATCH v1 2/4] clk: rockchip: add dt-binding header for rk3568
Hi Elaine, Am Mittwoch, 24. Februar 2021, 07:35:30 CET schrieb elaine.zhang: > Hi, Heiko: > > 在 2021/2/23 下午6:45, Heiko Stübner 写道: > > Hi Elaine, > > > > Am Dienstag, 23. Februar 2021, 10:53:50 CET schrieb Elaine Zhang: > >> Add the dt-bindings header for the rk3568, that gets shared between > >> the clock controller and the clock references in the dts. > >> Add softreset ID for rk3568. > >> > >> Signed-off-by: Elaine Zhang > >> --- > >> include/dt-bindings/clock/rk3568-cru.h | 926 + > >> 1 file changed, 926 insertions(+) > >> create mode 100644 include/dt-bindings/clock/rk3568-cru.h > >> > >> diff --git a/include/dt-bindings/clock/rk3568-cru.h > >> b/include/dt-bindings/clock/rk3568-cru.h > >> new file mode 100644 > >> index ..22b0b8739b5d > >> --- /dev/null > >> +++ b/include/dt-bindings/clock/rk3568-cru.h > >> @@ -0,0 +1,926 @@ > >> +/* SPDX-License-Identifier: GPL-2.0 */ > >> +/* > >> + * Copyright (c) 2021 Rockchip Electronics Co. Ltd. > >> + * Author: Elaine Zhang > >> + */ > >> + > >> +#ifndef _DT_BINDINGS_CLK_ROCKCHIP_RK3568_H > >> +#define _DT_BINDINGS_CLK_ROCKCHIP_RK3568_H > >> + > >> +/* pmucru-clocks indices */ > >> + > >> +/* pmucru plls */ > >> +#define PLL_PPLL 1 > >> +#define PLL_HPLL 2 > >> + > >> +/* pmucru clocks */ > >> +#define XIN_OSC0_DIV 4 > >> +#define CLK_RTC_32K 5 > >> +#define CLK_PMU 6 > >> +#define CLK_I2C0 7 > > can we change the prefix of CLK_* ids to SCLK_* > > (for special clock), like on previous socs. > > > > Especially as some of them already have that SCLK_prefix already anyway. > > > > Having that 4-letter prefix makes reading these IDs easier as well ;-) > > SCLK is for special clock, CLK is for common clock. > > rk3568-cru.h is automatically generated from TRM using tools. > Can we minimize the work of manual modification? > Because of the increasing number of clocks, writing by hand often makes > mistakes.We use tools to generate rk3568-cru.h(100% use tools) and generate > descriptions of registers in clk-rk3568.c(50% use tools) ok, sounds good. I didn't realize you're using tools now, so yes the autogenerated header can stay as it is then. Heiko > > > > > > > Thanks > > Heiko > > > >> +#define CLK_RTC32K_FRAC 8 > >> +#define CLK_UART0_DIV 9 > >> +#define CLK_UART0_FRAC10 > >> +#define SCLK_UART011 > >> +#define DBCLK_GPIO0 12 > >> +#define CLK_PWM0 13 > >> +#define CLK_CAPTURE_PWM0_NDFT 14 > >> +#define CLK_PMUPVTM 15 > >> +#define CLK_CORE_PMUPVTM 16 > >> +#define CLK_REF24M17 > >> +#define XIN_OSC0_USBPHY0_G18 > >> +#define CLK_USBPHY0_REF 19 > >> +#define XIN_OSC0_USBPHY1_G20 > >> +#define CLK_USBPHY1_REF 21 > >> +#define XIN_OSC0_MIPIDSIPHY0_G22 > >> +#define CLK_MIPIDSIPHY0_REF 23 > >> +#define XIN_OSC0_MIPIDSIPHY1_G24 > >> +#define CLK_MIPIDSIPHY1_REF 25 > >> +#define CLK_WIFI_DIV 26 > >> +#define CLK_WIFI_OSC0 27 > >> +#define CLK_WIFI 28 > >> +#define CLK_PCIEPHY0_DIV 29 > >> +#define CLK_PCIEPHY0_OSC0 30 > >> +#define CLK_PCIEPHY0_REF 31 > >> +#define CLK_PCIEPHY1_DIV 32 > >> +#define CLK_PCIEPHY1_OSC0 33 > >> +#define CLK_PCIEPHY1_REF 34 > >> +#define CLK_PCIEPHY2_DIV 35 > >> +#define CLK_PCIEPHY2_OSC0 36 > >> +#define CLK_PCIEPHY2_REF 37 > >> +#define CLK_PCIE30PHY_REF_M 38 > >> +#define CLK_PCIE30PHY_REF_N 39 > >> +#define CLK_HDMI_REF 40 > >> +#define XIN_OSC0_EDPPHY_G 41 > >> +#define PCLK_PDPMU42 > >> +#define PCLK_PMU 43 > >> +#define PCLK_UART044 > >> +#define PCLK_I2C0 45 > >> +#define PCLK_GPIO046 > >> +#define PCLK_PMUPVTM 47 > >> +#define PCLK_PWM0 48 > >> +#define CLK_PDPMU 49 > >> +#define SCLK_32K_IOE 50 > >> + > >> +#define CLKPMU_NR_CLKS(SCLK_32K_IOE + 1) > >> + > >> +/* cru-clocks indices */ >
Re: [PATCH v1 1/4] dt-bindings: add bindings for rk3568 clock controller
Am Dienstag, 23. Februar 2021, 11:50:25 CET schrieb Johan Jonker:
> Hi Elaine,
>
> This is a new document.
> Could you convert rockchip,rk3568-cru.txt to yaml?
I'll definitly second that wish for a conversion to yaml.
Having the ability to check devicetrees for correctness is
quite helpful :-)
Heiko
> To get an acked-by you must include:
>
> robh...@kernel.org
> devicet...@vger.kernel.org
>
> ./scripts/get_maintainer.pl --noroles --norolestats --nogit-fallback
> --nogit
>
> Your patch should show up here after filtering:
> https://patchwork.ozlabs.org/project/devicetree-bindings/list/
>
> Check with:
>
> make ARCH=arm64 dt_binding_check
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/pinctrl/rockchip,rk3568-cru.yaml
>
> make ARCH=arm64 dtbs_check
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/pinctrl/rockchip,rk3568-cru.yaml
>
> ./scripts/checkpatch.pl --strict
>
>
> On 2/23/21 10:53 AM, Elaine Zhang wrote:
> > Add devicetree bindings for Rockchip cru which found on
> > Rockchip SoCs.
> >
> > Signed-off-by: Elaine Zhang
> > ---
> > .../bindings/clock/rockchip,rk3568-cru.txt| 66 +++
> > 1 file changed, 66 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/clock/rockchip,rk3568-cru.txt
> >
> > diff --git
> > a/Documentation/devicetree/bindings/clock/rockchip,rk3568-cru.txt
> > b/Documentation/devicetree/bindings/clock/rockchip,rk3568-cru.txt
> > new file mode 100644
> > index ..b1119aecb7c7
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/rockchip,rk3568-cru.txt
> > @@ -0,0 +1,66 @@
> > +* Rockchip RK3568 Clock and Reset Unit
> > +
> > +The RK3568 clock controller generates and supplies clock to various
> > +controllers within the SoC and also implements a reset controller for SoC
> > +peripherals.
> > +
> > +Required Properties:
> > +
> > +- compatible: PMU for CRU should be "rockchip,rk3568-pmucru"
> > +- compatible: CRU should be "rockchip,rk3568-cru"
> > +- reg: physical base address of the controller and length of memory mapped
> > + region.
> > +- #clock-cells: should be 1.
> > +- #reset-cells: should be 1.
> > +
> > +Optional Properties:
> > +
> > +- rockchip,grf: phandle to the syscon managing the "general register files"
> > + If missing, pll rates are not changeable, due to the missing pll lock
> > status.
> > +
> > +Each clock is assigned an identifier and client nodes can use this
> > identifier
> > +to specify the clock which they consume. All available clocks are defined
> > as
> > +preprocessor macros in the dt-bindings/clock/rk3568-cru.h headers and can
> > be
> > +used in device tree sources. Similar macros exist for the reset sources in
> > +these files.
> > +
> > +External clocks:
> > +
> > +There are several clocks that are generated outside the SoC. It is expected
> > +that they are defined using standard clock bindings with following
> > +clock-output-names:
> > + - "xin24m" - crystal input - required,
> > + - "xin32k" - rtc clock - optional,
> > + - "i2sx_mclkin" - external I2S clock - optional,
> > + - "xin_osc0_usbphyx_g" - external USBPHY clock - optional,
> > + - "xin_osc0_mipidsiphyx_g" - external MIPIDSIPHY clock - optional,
> > +
> > +Example: Clock controller node:
> > +
> > + pmucru: clock-controller@fdd0 {
> > + compatible = "rockchip,rK3568-pmucru";
> > + reg = <0x0 0xfdd0 0x0 0x1000>;
> > + #clock-cells = <1>;
> > + #reset-cells = <1>;
> > + };
> > +
> > + cru: clock-controller@fdd2 {
> > + compatible = "rockchip,rK3568-cru";
> > + reg = <0x0 0xfdd2 0x0 0x1000>;
> > + rockchip,grf = <>;
> > + #clock-cells = <1>;
> > + #reset-cells = <1>;
> > + };
> > +
> > +Example: UART controller node that consumes the clock generated by the
> > clock
> > + controller:
> > +
> > + uart1: serial@fe65 {
> > + compatible = "rockchip,rK3568-uart", "snps,dw-apb-uart";
> > + reg = <0x0 0xfe65 0x0 0x100>;
> > + interrupts = ;
> > + reg-shift = <2>;
> > + reg-io-width = <4>;
> > + clocks = < SCLK_UART1>, < PCLK_UART1>;
> > + clock-names = "baudclk", "apb_pclk";
> > + };
> >
>
>
Re: [PATCH v1 2/4] clk: rockchip: add dt-binding header for rk3568
Hi Elaine, Am Dienstag, 23. Februar 2021, 10:53:50 CET schrieb Elaine Zhang: > Add the dt-bindings header for the rk3568, that gets shared between > the clock controller and the clock references in the dts. > Add softreset ID for rk3568. > > Signed-off-by: Elaine Zhang > --- > include/dt-bindings/clock/rk3568-cru.h | 926 + > 1 file changed, 926 insertions(+) > create mode 100644 include/dt-bindings/clock/rk3568-cru.h > > diff --git a/include/dt-bindings/clock/rk3568-cru.h > b/include/dt-bindings/clock/rk3568-cru.h > new file mode 100644 > index ..22b0b8739b5d > --- /dev/null > +++ b/include/dt-bindings/clock/rk3568-cru.h > @@ -0,0 +1,926 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (c) 2021 Rockchip Electronics Co. Ltd. > + * Author: Elaine Zhang > + */ > + > +#ifndef _DT_BINDINGS_CLK_ROCKCHIP_RK3568_H > +#define _DT_BINDINGS_CLK_ROCKCHIP_RK3568_H > + > +/* pmucru-clocks indices */ > + > +/* pmucru plls */ > +#define PLL_PPLL 1 > +#define PLL_HPLL 2 > + > +/* pmucru clocks */ > +#define XIN_OSC0_DIV 4 > +#define CLK_RTC_32K 5 > +#define CLK_PMU 6 > +#define CLK_I2C0 7 can we change the prefix of CLK_* ids to SCLK_* (for special clock), like on previous socs. Especially as some of them already have that SCLK_prefix already anyway. Having that 4-letter prefix makes reading these IDs easier as well ;-) Thanks Heiko > +#define CLK_RTC32K_FRAC 8 > +#define CLK_UART0_DIV9 > +#define CLK_UART0_FRAC 10 > +#define SCLK_UART0 11 > +#define DBCLK_GPIO0 12 > +#define CLK_PWM0 13 > +#define CLK_CAPTURE_PWM0_NDFT14 > +#define CLK_PMUPVTM 15 > +#define CLK_CORE_PMUPVTM 16 > +#define CLK_REF24M 17 > +#define XIN_OSC0_USBPHY0_G 18 > +#define CLK_USBPHY0_REF 19 > +#define XIN_OSC0_USBPHY1_G 20 > +#define CLK_USBPHY1_REF 21 > +#define XIN_OSC0_MIPIDSIPHY0_G 22 > +#define CLK_MIPIDSIPHY0_REF 23 > +#define XIN_OSC0_MIPIDSIPHY1_G 24 > +#define CLK_MIPIDSIPHY1_REF 25 > +#define CLK_WIFI_DIV 26 > +#define CLK_WIFI_OSC027 > +#define CLK_WIFI 28 > +#define CLK_PCIEPHY0_DIV 29 > +#define CLK_PCIEPHY0_OSC030 > +#define CLK_PCIEPHY0_REF 31 > +#define CLK_PCIEPHY1_DIV 32 > +#define CLK_PCIEPHY1_OSC033 > +#define CLK_PCIEPHY1_REF 34 > +#define CLK_PCIEPHY2_DIV 35 > +#define CLK_PCIEPHY2_OSC036 > +#define CLK_PCIEPHY2_REF 37 > +#define CLK_PCIE30PHY_REF_M 38 > +#define CLK_PCIE30PHY_REF_N 39 > +#define CLK_HDMI_REF 40 > +#define XIN_OSC0_EDPPHY_G41 > +#define PCLK_PDPMU 42 > +#define PCLK_PMU 43 > +#define PCLK_UART0 44 > +#define PCLK_I2C045 > +#define PCLK_GPIO0 46 > +#define PCLK_PMUPVTM 47 > +#define PCLK_PWM048 > +#define CLK_PDPMU49 > +#define SCLK_32K_IOE 50 > + > +#define CLKPMU_NR_CLKS (SCLK_32K_IOE + 1) > + > +/* cru-clocks indices */ > + > +/* cru plls */ > +#define PLL_APLL 1 > +#define PLL_DPLL 2 > +#define PLL_CPLL 3 > +#define PLL_GPLL 4 > +#define PLL_VPLL 5 > +#define PLL_NPLL 6 > + > +/* cru clocks */ > +#define CPLL_333M9 > +#define ARMCLK 10 > +#define USB480M 11 > +#define ACLK_CORE_NIU2BUS18 > +#define CLK_CORE_PVTM19 > +#define CLK_CORE_PVTM_CORE 20 > +#define CLK_CORE_PVTPLL 21 > +#define CLK_GPU_SRC 22 > +#define CLK_GPU_PRE_NDFT 23 > +#define CLK_GPU_PRE_MUX 24 > +#define ACLK_GPU_PRE 25 > +#define PCLK_GPU_PRE 26 > +#define CLK_GPU 27 > +#define CLK_GPU_NP5 28 > +#define PCLK_GPU_PVTM29 > +#define CLK_GPU_PVTM 30 > +#define CLK_GPU_PVTM_CORE31 > +#define CLK_GPU_PVTPLL 32 > +#define CLK_NPU_SRC 33 > +#define CLK_NPU_PRE_NDFT 34 > +#define CLK_NPU 35 > +#define CLK_NPU_NP5 36 > +#define HCLK_NPU_PRE 37 > +#define PCLK_NPU_PRE 38 > +#define ACLK_NPU_PRE 39 > +#define ACLK_NPU 40 > +#define HCLK_NPU 41 > +#define PCLK_NPU_PVTM42 > +#define CLK_NPU_PVTM 43 > +#define CLK_NPU_PVTM_CORE44 > +#define CLK_NPU_PVTPLL 45 > +#define CLK_DDRPHY1X_SRC 46 > +#define CLK_DDRPHY1X_HWFFC_SRC 47 > +#define CLK_DDR1X48 > +#define CLK_MSCH 49 > +#define CLK24_DDRMON 50 > +#define ACLK_GIC_AUDIO 51 > +#define HCLK_GIC_AUDIO 52 > +#define HCLK_SDMMC_BUFFER53 > +#define DCLK_SDMMC_BUFFER54 > +#define ACLK_GIC600 55 > +#define ACLK_SPINLOCK56 >
Re: [PATCH v1 3/4] clk: rockchip: support more core div setting
Hi Elaine,
Am Dienstag, 23. Februar 2021, 10:53:51 CET schrieb Elaine Zhang:
> A55 supports each core to work at different frequencies, and each core
> has an independent divider control.
>
> Signed-off-by: Elaine Zhang
> ---
> drivers/clk/rockchip/clk-cpu.c | 25 +
> drivers/clk/rockchip/clk.h | 17 -
> 2 files changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/rockchip/clk-cpu.c b/drivers/clk/rockchip/clk-cpu.c
> index fa9027fb1920..cac06f4f7573 100644
> --- a/drivers/clk/rockchip/clk-cpu.c
> +++ b/drivers/clk/rockchip/clk-cpu.c
> @@ -164,6 +164,18 @@ static int rockchip_cpuclk_pre_rate_change(struct
> rockchip_cpuclk *cpuclk,
>reg_data->mux_core_mask,
>reg_data->mux_core_shift),
> cpuclk->reg_base + reg_data->core_reg);
> + if (reg_data->core1_reg)
> + writel(HIWORD_UPDATE(alt_div, reg_data->div_core1_mask,
> + reg_data->div_core1_shift),
> +cpuclk->reg_base + reg_data->core1_reg);
> + if (reg_data->core2_reg)
> + writel(HIWORD_UPDATE(alt_div, reg_data->div_core2_mask,
> + reg_data->div_core2_shift),
> +cpuclk->reg_base + reg_data->core2_reg);
> + if (reg_data->core3_reg)
> + writel(HIWORD_UPDATE(alt_div, reg_data->div_core3_mask,
> + reg_data->div_core3_shift),
> +cpuclk->reg_base + reg_data->core3_reg);
for (i = 0; i < reg_data->num_cores; i++)
writel(...)
> } else {
> /* select alternate parent */
> writel(HIWORD_UPDATE(reg_data->mux_core_alt,
> @@ -209,6 +221,19 @@ static int rockchip_cpuclk_post_rate_change(struct
> rockchip_cpuclk *cpuclk,
> reg_data->mux_core_shift),
> cpuclk->reg_base + reg_data->core_reg);
>
> + if (reg_data->core1_reg)
> + writel(HIWORD_UPDATE(0, reg_data->div_core1_mask,
> + reg_data->div_core1_shift),
> +cpuclk->reg_base + reg_data->core1_reg);
> + if (reg_data->core2_reg)
> + writel(HIWORD_UPDATE(0, reg_data->div_core2_mask,
> + reg_data->div_core2_shift),
> +cpuclk->reg_base + reg_data->core2_reg);
> + if (reg_data->core3_reg)
> + writel(HIWORD_UPDATE(0, reg_data->div_core3_mask,
> + reg_data->div_core3_shift),
> +cpuclk->reg_base + reg_data->core3_reg);
> +
for (i = 0; i < reg_data->num_cores; i++)
writel(...)
> if (ndata->old_rate > ndata->new_rate)
> rockchip_cpuclk_set_dividers(cpuclk, rate);
>
> diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h
> index 2271a84124b0..b46c93fd0cb5 100644
> --- a/drivers/clk/rockchip/clk.h
> +++ b/drivers/clk/rockchip/clk.h
> @@ -322,7 +322,7 @@ struct rockchip_cpuclk_clksel {
> u32 val;
> };
>
> -#define ROCKCHIP_CPUCLK_NUM_DIVIDERS 2
> +#define ROCKCHIP_CPUCLK_NUM_DIVIDERS 5
please move this into a separate patch, as yes the rk3568 needs more
dividers but that isn't related to adding separate core divider controls.
[...]
add
#define ROCKCHIP_CPUCLK_MAX_CORES 4
> struct rockchip_cpuclk_rate_table {
> unsigned long prate;
> struct rockchip_cpuclk_clksel divs[ROCKCHIP_CPUCLK_NUM_DIVIDERS];
> @@ -333,6 +333,12 @@ struct rockchip_cpuclk_rate_table {
> * @core_reg:register offset of the core settings register
> * @div_core_shift: core divider offset used to divide the pll value
> * @div_core_mask: core divider mask
> + * @div_core1_shift: core1 divider offset used to divide the pll value
> + * @div_core1_mask: core1 divider mask
> + * @div_core2_shift: core2 divider offset used to divide the pll value
> + * @div_core2_mask: core2 divider mask
> + * @div_core3_shift: core3 divider offset used to divide the pll value
> + * @div_core3_mask: core3 divider mask
> * @mux_core_alt:mux value to select alternate parent
> * @mux_core_main: mux value to select main parent of core
> * @mux_core_shift: offset of the core multiplexer
> @@ -342,6 +348,15 @@ struct rockchip_cpuclk_reg_data {
> int core_reg;
> u8 div_core_shift;
> u32 div_core_mask;
> + int core1_reg;
> + u8 div_core1_shift;
> + u32 div_core1_mask;
> + int core2_reg;
> + u8 div_core2_shift;
> + u32 div_core2_mask;
> + int core3_reg;
> + u8 div_core3_shift;
> + u32 div_core3_mask;
please make this instead like:
int
Re: [PATCH 0/6] Support second Image Signal Processor on rk3399
Hi Sebastian, Am Donnerstag, 11. Februar 2021, 06:25:15 CET schrieb Sebastian Fricke: > Hey Heiko, > > On 10.02.2021 12:15, Heiko Stübner wrote: > >Hi Sebastian, > > > >Am Freitag, 5. Februar 2021, 15:55:56 CET schrieb Heiko Stübner: > >> Hi Sebastian, > >> > >> I did some tests myself today as well and can confirm your > >> hdmi related finding - at least when plugged in on boot. > >> > >> I tried some combinations of camera vs. hdmi and it seems > >> really only when hdmi is plugged in on boot > > > >as you can see in v2, it should work now even with hdmi > >connected on boot. My patch ignored the grf-clock when > >doing the grf-based init. > > > >All clocks are on during boot and I guess the hdmi-driver > >did disable it after its probe. The phy_power_on functions > >did handle it correctly already, so it was only happening > >with hdmi connected on boot. > > Thank you very much for solving that problem, I've tested the scenarios > described below and it works like a charm. (With your V2) > > > > > >Btw. do you plan on submitting your ov13850 driver > >soonish? > > Actually, I have posted the patch already see here: > https://patchwork.kernel.org/project/linux-media/patch/20210130182313.32903-2-sebastian.fri...@posteo.net/ very cool to see > I currently review the requested changes and questions and will soon > post a second version, but I expect quite some time until it is actually > merged. could you Cc me on future versions? Thanks Heiko > > Greetings, > Sebastian > > > > > > >> > >> (1) > >> - boot > >> - camera > >> --> works > >> > >> (2) > >> - boot > >> - camera > >> - hdmi plugged in > >> - hdmi works > >> - camera > >> --> works > >> > >> (3) > >> - hdmi plugged in > >> - boot > >> - hdmi works > >> - camera > >> --> camera doesn't work > >> > >> (4) > >> - boot > >> - hdmi plugged in > >> - hdmi works > >> - camera > >> -> camera works > >> > >> > >> With a bit of brute-force [0] it seems the camera also works again even > >> with hdmi connected on boot. So conclusion would be that some clock > >> is misbehaving. > >> > >> Now we'll "only" need to find out which one that is. > >> > >> > >> Heiko > >> > >> > >> [0] > >> Don't disable any clock gates > >> > >> diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c > >> index 070dc47e95a1..8daf1fc3388c 100644 > >> --- a/drivers/clk/clk-gate.c > >> +++ b/drivers/clk/clk-gate.c > >> @@ -61,6 +61,9 @@ static void clk_gate_endisable(struct clk_hw *hw, int > >> enable) > >> > >> set ^= enable; > >> > >> +if (!enable) > >> +return; > >> + > >> if (gate->lock) > >> spin_lock_irqsave(gate->lock, flags); > >> else > >> > >> > >> > >> Am Freitag, 5. Februar 2021, 09:15:47 CET schrieb Heiko Stübner: > >> > Hi Sebastian, > >> > > >> > Am Freitag, 5. Februar 2021, 07:43:35 CET schrieb Sebastian Fricke: > >> > > On 03.02.2021 20:54, Heiko Stübner wrote: > >> > > >Am Mittwoch, 3. Februar 2021, 19:14:22 CET schrieb Sebastian Fricke: > >> > > >> I have tested your patch set on my nanoPC-T4, here is a complete log > >> > > >> with: > >> > > >> - relevant kernel log entries > >> > > >> - system information > >> > > >> - media ctl output > >> > > >> - sysfs entry information > >> > > >> > >> > > >> https://paste.debian.net/1183874/ > >> > > >> > >> > > >> Additionally, to your patchset I have applied the following patches: > >> > > >> https://github.com/initBasti/Linux_kernel_media_tree_fork/commits/dual_cam_setup > >> > > >> > >> > > >> And just to not cause confusion the `media_dev` entries come from > >> > > >> this > >> > > >> unmerged series: > >> > > >> https://patchwork.kernel.org/project/linux-media/list/?series=426269 > >> > > >> > >> > > >> I have actually been able to stream
Re: [PATCH 0/6] Support second Image Signal Processor on rk3399
Hi Sebastian, Am Freitag, 5. Februar 2021, 15:55:56 CET schrieb Heiko Stübner: > Hi Sebastian, > > I did some tests myself today as well and can confirm your > hdmi related finding - at least when plugged in on boot. > > I tried some combinations of camera vs. hdmi and it seems > really only when hdmi is plugged in on boot as you can see in v2, it should work now even with hdmi connected on boot. My patch ignored the grf-clock when doing the grf-based init. All clocks are on during boot and I guess the hdmi-driver did disable it after its probe. The phy_power_on functions did handle it correctly already, so it was only happening with hdmi connected on boot. Btw. do you plan on submitting your ov13850 driver soonish? Heiko > > (1) > - boot > - camera > --> works > > (2) > - boot > - camera > - hdmi plugged in > - hdmi works > - camera > --> works > > (3) > - hdmi plugged in > - boot > - hdmi works > - camera > --> camera doesn't work > > (4) > - boot > - hdmi plugged in > - hdmi works > - camera > -> camera works > > > With a bit of brute-force [0] it seems the camera also works again even > with hdmi connected on boot. So conclusion would be that some clock > is misbehaving. > > Now we'll "only" need to find out which one that is. > > > Heiko > > > [0] > Don't disable any clock gates > > diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c > index 070dc47e95a1..8daf1fc3388c 100644 > --- a/drivers/clk/clk-gate.c > +++ b/drivers/clk/clk-gate.c > @@ -61,6 +61,9 @@ static void clk_gate_endisable(struct clk_hw *hw, int > enable) > > set ^= enable; > > +if (!enable) > +return; > + > if (gate->lock) > spin_lock_irqsave(gate->lock, flags); > else > > > > Am Freitag, 5. Februar 2021, 09:15:47 CET schrieb Heiko Stübner: > > Hi Sebastian, > > > > Am Freitag, 5. Februar 2021, 07:43:35 CET schrieb Sebastian Fricke: > > > On 03.02.2021 20:54, Heiko Stübner wrote: > > > >Am Mittwoch, 3. Februar 2021, 19:14:22 CET schrieb Sebastian Fricke: > > > >> I have tested your patch set on my nanoPC-T4, here is a complete log > > > >> with: > > > >> - relevant kernel log entries > > > >> - system information > > > >> - media ctl output > > > >> - sysfs entry information > > > >> > > > >> https://paste.debian.net/1183874/ > > > >> > > > >> Additionally, to your patchset I have applied the following patches: > > > >> https://github.com/initBasti/Linux_kernel_media_tree_fork/commits/dual_cam_setup > > > >> > > > >> And just to not cause confusion the `media_dev` entries come from this > > > >> unmerged series: > > > >> https://patchwork.kernel.org/project/linux-media/list/?series=426269 > > > >> > > > >> I have actually been able to stream with both of my cameras at the same > > > >> time using the libcamera cam command. > > > >> I would like to thank you a lot for making this possible. > > > > > > > >Thanks for testing a dual camera setup. On my board I could only test > > > >the second ISP. And really glad it works for you tool :-) . > > > > > > > >Out of curiosity, do you also see that green tint in the images the > > > >cameras > > > >produce? > > > > > > Yes, I do. Actually, I currently have two forms of a green tint, on my > > > OV13850 everything is quite dark and greenish, which is caused by the > > > missing 3A algorithms. On my OV4689, I have big patches of the image > > > with bright green color and flickering, I investigated if this is > > > connected to the 2nd ISP instance, but that doesn't seem to be the case > > > as I have the same results when I switch the CSI ports of the cameras. > > > > > > I have found another issue, while testing I discovered following > > > issue: > > > When I start the system with an HDMI monitor connected, then the camera > > > on the 2nd port doesn't work. This is probably because the RX/TX is > > > reserved as a TX. > > > But it made me wonder because if the system has an RX, a TX, and > > > an RX/TX, why isn't the pure TX used by the monitor and the > > > cameras take RX and RX/TX? > > > Or do you think that this is maybe a malfunction of this patch? > > > > I don't think it is an issue with this specific s
Re: [PATCH v2 0/6] Support second Image Signal Processor on rk3399
Am Mittwoch, 10. Februar 2021, 12:10:14 CET schrieb Heiko Stuebner: > The rk3399 has two ISPs and right now only the first one is usable. > The second ISP is connected to the TXRX dphy on the soc. > > The phy of ISP1 is only accessible through the DSI controller's > io-memory, so this series adds support for simply using the dsi > controller is a phy if needed. > > That solution is needed at least on rk3399 and rk3288 but no-one > has looked at camera support on rk3288 at all, so right now > only implement the rk3399 specifics. > > changes in v2: > - enable grf-clock also for init callback > to not break if for example hdmi is connected on boot > and disabled the grf clock during its probe > - add Sebastian's Tested-by > - add Rob's Ack for the phy-cells property > > Heiko Stuebner (6): > drm/rockchip: dsi: add own additional pclk handling > dt-bindings: display: rockchip-dsi: add optional #phy-cells property > drm/rockchip: dsi: add ability to work as a phy instead of full dsi > arm64: dts: rockchip: add #phy-cells to mipi-dsi1 > arm64: dts: rockchip: add cif clk-control pinctrl for rk3399 > arm64: dts: rockchip: add isp1 node on rk3399 of course everything was meant to be v2 in the subject. Heiko
Re: [PATCH 2/5] ARM: dts: rockchip: assign a fixed index to mmc devices on rv1108 boards
Am Mittwoch, 10. Februar 2021, 11:34:41 CET schrieb Arnd Bergmann:
> On Wed, Feb 10, 2021 at 12:50 AM Heiko Stübner wrote:
> > Am Dienstag, 9. Februar 2021, 23:25:40 CET schrieb Arnd Bergmann:
> >
> > Hmm, right now I don't see the disadvantage of missing mmc numbers.
>
> It's inconsistent with the normal use of these aliases across other
> platforms.
>
> > As similarly we count i2c and serial numbers for a long time, even though
> > not all of them appear on every board.
>
> Yes, that is a similar mistake.
>
> > Especially as the main goal is to simply have stable numbers and
> > not having the mmc devices swap numbers on every boot.
> >
> > So right now we're not using them from a userspace POV but
> > instead agreed on following the address ordering of the soc.
> > so when ordering mmc controllers by their io-address, mmc0
> > is the first one, then mmc1, etc.
> >
> > So just for my understanding, what is different for mmc?
> > I guess to guarantee ongoing numbering similar to sd{a,b,c,...}
> > Or should all aliases be duplicted in each board dts and not
> > live in any soc dtsi?
>
> Each board should have its own aliases node that describes
> exactly which of the devices are wired up on that board, and
> in which order. If there are connectors on the board that
> are labeled in some form, then the aliases are meant to
> match what is written on the board or in its documentation.
Then we're at least in the clear for i2c, serial and the rest ... as these
are numbered in the soc documentation, and all boards I've seen so
far use these number also to identify these in schematics.
So an i2c2 is always i2c2 even if i2c1 is not populated.
And of course doing
i2c0 =
would definitly confuse people to no end.
So I guess we'll just move the mmc stuff over to board files.
Re: [PATCH 2/5] ARM: dts: rockchip: assign a fixed index to mmc devices on rv1108 boards
Am Dienstag, 9. Februar 2021, 23:25:40 CET schrieb Arnd Bergmann:
> On Mon, Jan 18, 2021 at 4:52 PM Johan Jonker wrote:
> >
> > Recently introduced async probe on mmc devices can shuffle block IDs.
> > Pin them to fixed values to ease booting in environments where UUIDs are
> > not practical. Use newly introduced aliases for mmcblk devices from [1].
> > The sort order is based on reg address.
> >
> > [1] https://patchwork.kernel.org/patch/11747669/
>
> I just saw this in the pull request:
>
> > Signed-off-by: Johan Jonker
> > ---
> > arch/arm/boot/dts/rv1108.dtsi | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/rv1108.dtsi b/arch/arm/boot/dts/rv1108.dtsi
> > index bec47e0be..a754851f4 100644
> > --- a/arch/arm/boot/dts/rv1108.dtsi
> > +++ b/arch/arm/boot/dts/rv1108.dtsi
> > @@ -19,6 +19,9 @@
> > i2c1 =
> > i2c2 =
> > i2c3 =
> > + mmc0 =
> > + mmc1 =
> > + mmc2 =
> > serial0 =
> > serial1 =
> > serial2 =
>
> Please don't put these aliases into a .dtsi file, as not every board
> will provide each instance. The entire point of the aliases is to
> have sane enumeration, so you should start at index 0 for the
> first one that is actually present and count up from there.
Hmm, right now I don't see the disadvantage of missing mmc numbers.
As similarly we count i2c and serial numbers for a long time, even though
not all of them appear on every board.
Especially as the main goal is to simply have stable numbers and
not having the mmc devices swap numbers on every boot.
So right now we're not using them from a userspace POV but
instead agreed on following the address ordering of the soc.
so when ordering mmc controllers by their io-address, mmc0
is the first one, then mmc1, etc.
So just for my understanding, what is different for mmc?
I guess to guarantee ongoing numbering similar to sd{a,b,c,...}
Or should all aliases be duplicted in each board dts and not
live in any soc dtsi?
Heiko
> I would suggest you move these aliases into the .dts files for
> the existing boards for the next cycle, and then make sure
> only the ones that are present have an alias.
>
> It might actually be a good idea to have a warning in dtc when
> there is an alias pointing to a status="disabled" device, but I
> suspect there would be a lot of fallout from that.
>
> Arnd
>
Re: [PATCH 0/6] Support second Image Signal Processor on rk3399
Hi Sebastian, I did some tests myself today as well and can confirm your hdmi related finding - at least when plugged in on boot. I tried some combinations of camera vs. hdmi and it seems really only when hdmi is plugged in on boot (1) - boot - camera --> works (2) - boot - camera - hdmi plugged in - hdmi works - camera --> works (3) - hdmi plugged in - boot - hdmi works - camera --> camera doesn't work (4) - boot - hdmi plugged in - hdmi works - camera -> camera works With a bit of brute-force [0] it seems the camera also works again even with hdmi connected on boot. So conclusion would be that some clock is misbehaving. Now we'll "only" need to find out which one that is. Heiko [0] Don't disable any clock gates diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c index 070dc47e95a1..8daf1fc3388c 100644 --- a/drivers/clk/clk-gate.c +++ b/drivers/clk/clk-gate.c @@ -61,6 +61,9 @@ static void clk_gate_endisable(struct clk_hw *hw, int enable) set ^= enable; +if (!enable) +return; + if (gate->lock) spin_lock_irqsave(gate->lock, flags); else Am Freitag, 5. Februar 2021, 09:15:47 CET schrieb Heiko Stübner: > Hi Sebastian, > > Am Freitag, 5. Februar 2021, 07:43:35 CET schrieb Sebastian Fricke: > > On 03.02.2021 20:54, Heiko Stübner wrote: > > >Am Mittwoch, 3. Februar 2021, 19:14:22 CET schrieb Sebastian Fricke: > > >> I have tested your patch set on my nanoPC-T4, here is a complete log > > >> with: > > >> - relevant kernel log entries > > >> - system information > > >> - media ctl output > > >> - sysfs entry information > > >> > > >> https://paste.debian.net/1183874/ > > >> > > >> Additionally, to your patchset I have applied the following patches: > > >> https://github.com/initBasti/Linux_kernel_media_tree_fork/commits/dual_cam_setup > > >> > > >> And just to not cause confusion the `media_dev` entries come from this > > >> unmerged series: > > >> https://patchwork.kernel.org/project/linux-media/list/?series=426269 > > >> > > >> I have actually been able to stream with both of my cameras at the same > > >> time using the libcamera cam command. > > >> I would like to thank you a lot for making this possible. > > > > > >Thanks for testing a dual camera setup. On my board I could only test > > >the second ISP. And really glad it works for you tool :-) . > > > > > >Out of curiosity, do you also see that green tint in the images the cameras > > >produce? > > > > Yes, I do. Actually, I currently have two forms of a green tint, on my > > OV13850 everything is quite dark and greenish, which is caused by the > > missing 3A algorithms. On my OV4689, I have big patches of the image > > with bright green color and flickering, I investigated if this is > > connected to the 2nd ISP instance, but that doesn't seem to be the case > > as I have the same results when I switch the CSI ports of the cameras. > > > > I have found another issue, while testing I discovered following > > issue: > > When I start the system with an HDMI monitor connected, then the camera > > on the 2nd port doesn't work. This is probably because the RX/TX is > > reserved as a TX. > > But it made me wonder because if the system has an RX, a TX, and > > an RX/TX, why isn't the pure TX used by the monitor and the > > cameras take RX and RX/TX? > > Or do you think that this is maybe a malfunction of this patch? > > I don't think it is an issue with this specific series, but still puzzling. > > I.e. the DPHYs are actually only relevant to the DSI controllers, > with TX0 being connected to DSI0 and TX1RX1 being connected > to DSI1. So having an hdmi display _in theory_ shouldn't matter at all. > > Out of curiosity what happens, when you boot without hdmi connected > turn on the cameras, connect the hdmi after this, try the cameras again? > > > Heiko > > > > > > > > >Thanks > > >Heiko > > > > Greetings, > > Sebastian > > > > > > > > > > >> If you like to you can add: > > >> Tested-by: Sebastian Fricke > > >> > > >> On 02.02.2021 15:56, Heiko Stuebner wrote: > > >> >The rk3399 has two ISPs and right now only the first one is usable. > > >> >The second ISP is connected to the TXRX dphy on the soc. > > >> > > > >> >The phy of ISP1 is only accessible through the DSI controller's > > >> >io-memory, so this series adds suppo
Re: [PATCH 0/6] Support second Image Signal Processor on rk3399
Hi Sebastian, Am Freitag, 5. Februar 2021, 07:43:35 CET schrieb Sebastian Fricke: > On 03.02.2021 20:54, Heiko Stübner wrote: > >Am Mittwoch, 3. Februar 2021, 19:14:22 CET schrieb Sebastian Fricke: > >> I have tested your patch set on my nanoPC-T4, here is a complete log > >> with: > >> - relevant kernel log entries > >> - system information > >> - media ctl output > >> - sysfs entry information > >> > >> https://paste.debian.net/1183874/ > >> > >> Additionally, to your patchset I have applied the following patches: > >> https://github.com/initBasti/Linux_kernel_media_tree_fork/commits/dual_cam_setup > >> > >> And just to not cause confusion the `media_dev` entries come from this > >> unmerged series: > >> https://patchwork.kernel.org/project/linux-media/list/?series=426269 > >> > >> I have actually been able to stream with both of my cameras at the same > >> time using the libcamera cam command. > >> I would like to thank you a lot for making this possible. > > > >Thanks for testing a dual camera setup. On my board I could only test > >the second ISP. And really glad it works for you tool :-) . > > > >Out of curiosity, do you also see that green tint in the images the cameras > >produce? > > Yes, I do. Actually, I currently have two forms of a green tint, on my > OV13850 everything is quite dark and greenish, which is caused by the > missing 3A algorithms. On my OV4689, I have big patches of the image > with bright green color and flickering, I investigated if this is > connected to the 2nd ISP instance, but that doesn't seem to be the case > as I have the same results when I switch the CSI ports of the cameras. > > I have found another issue, while testing I discovered following > issue: > When I start the system with an HDMI monitor connected, then the camera > on the 2nd port doesn't work. This is probably because the RX/TX is > reserved as a TX. > But it made me wonder because if the system has an RX, a TX, and > an RX/TX, why isn't the pure TX used by the monitor and the > cameras take RX and RX/TX? > Or do you think that this is maybe a malfunction of this patch? I don't think it is an issue with this specific series, but still puzzling. I.e. the DPHYs are actually only relevant to the DSI controllers, with TX0 being connected to DSI0 and TX1RX1 being connected to DSI1. So having an hdmi display _in theory_ shouldn't matter at all. Out of curiosity what happens, when you boot without hdmi connected turn on the cameras, connect the hdmi after this, try the cameras again? Heiko > > > > >Thanks > >Heiko > > Greetings, > Sebastian > > > > > > >> If you like to you can add: > >> Tested-by: Sebastian Fricke > >> > >> On 02.02.2021 15:56, Heiko Stuebner wrote: > >> >The rk3399 has two ISPs and right now only the first one is usable. > >> >The second ISP is connected to the TXRX dphy on the soc. > >> > > >> >The phy of ISP1 is only accessible through the DSI controller's > >> >io-memory, so this series adds support for simply using the dsi > >> >controller is a phy if needed. > >> > > >> >That solution is needed at least on rk3399 and rk3288 but no-one > >> >has looked at camera support on rk3288 at all, so right now > >> >only implement the rk3399 specifics. > >> > > >> > > >> >Heiko Stuebner (6): > >> > drm/rockchip: dsi: add own additional pclk handling > >> > dt-bindings: display: rockchip-dsi: add optional #phy-cells property > >> > drm/rockchip: dsi: add ability to work as a phy instead of full dsi > >> > arm64: dts: rockchip: add #phy-cells to mipi-dsi1 > >> > arm64: dts: rockchip: add cif clk-control pinctrl for rk3399 > >> > arm64: dts: rockchip: add isp1 node on rk3399 > >> > > >> > .../display/rockchip/dw_mipi_dsi_rockchip.txt | 1 + > >> > arch/arm64/boot/dts/rockchip/rk3399.dtsi | 39 ++ > >> > drivers/gpu/drm/rockchip/Kconfig | 2 + > >> > .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 342 ++ > >> > 4 files changed, 384 insertions(+) > >> > > >> > > > > > > > > >
Re: [PATCH 0/6] Support second Image Signal Processor on rk3399
Hi Sebastian, Am Mittwoch, 3. Februar 2021, 19:14:22 CET schrieb Sebastian Fricke: > Hey Heiko, > > I have tested your patch set on my nanoPC-T4, here is a complete log > with: > - relevant kernel log entries > - system information > - media ctl output > - sysfs entry information > > https://paste.debian.net/1183874/ > > Additionally, to your patchset I have applied the following patches: > https://github.com/initBasti/Linux_kernel_media_tree_fork/commits/dual_cam_setup > > And just to not cause confusion the `media_dev` entries come from this > unmerged series: > https://patchwork.kernel.org/project/linux-media/list/?series=426269 > > I have actually been able to stream with both of my cameras at the same > time using the libcamera cam command. > I would like to thank you a lot for making this possible. Thanks for testing a dual camera setup. On my board I could only test the second ISP. And really glad it works for you tool :-) . Out of curiosity, do you also see that green tint in the images the cameras produce? Thanks Heiko > If you like to you can add: > Tested-by: Sebastian Fricke > > On 02.02.2021 15:56, Heiko Stuebner wrote: > >The rk3399 has two ISPs and right now only the first one is usable. > >The second ISP is connected to the TXRX dphy on the soc. > > > >The phy of ISP1 is only accessible through the DSI controller's > >io-memory, so this series adds support for simply using the dsi > >controller is a phy if needed. > > > >That solution is needed at least on rk3399 and rk3288 but no-one > >has looked at camera support on rk3288 at all, so right now > >only implement the rk3399 specifics. > > > > > >Heiko Stuebner (6): > > drm/rockchip: dsi: add own additional pclk handling > > dt-bindings: display: rockchip-dsi: add optional #phy-cells property > > drm/rockchip: dsi: add ability to work as a phy instead of full dsi > > arm64: dts: rockchip: add #phy-cells to mipi-dsi1 > > arm64: dts: rockchip: add cif clk-control pinctrl for rk3399 > > arm64: dts: rockchip: add isp1 node on rk3399 > > > > .../display/rockchip/dw_mipi_dsi_rockchip.txt | 1 + > > arch/arm64/boot/dts/rockchip/rk3399.dtsi | 39 ++ > > drivers/gpu/drm/rockchip/Kconfig | 2 + > > .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 342 ++ > > 4 files changed, 384 insertions(+) > > >
Re: [PATCH] drm/bridge: dw-mipi-dsi: Move drm_bridge_add into probe
Am Mittwoch, 3. Februar 2021, 10:13:06 CET schrieb Jagan Teki:
> Usual I2C configured DSI bridge drivers have drm_bridge_add
> in probe and mipi_dsi_attach in bridge attach functions.
>
> With, this approach the drm pipeline is unable to find the
> dsi bridge in stm drm drivers since the dw-mipi-dsi bridge is
> adding drm bridge during bridge attach operations instead of
> the probe.
Shouldn't the STM drm driver not simply defer if it's missing
a bridge that is referenced in the devicetree or somewhere?
> This specific issue may not encounter for rockchip drm dsi
> drivers, since rockchip drm uses component binding operations,
> unlike stm drm drivers.
>
> So, possible solutions are
> 1. Move drm_bridge_add into the dw-mipi-dsi probe.
> 2. Add mipi_dsi_attach in the bridge drivers probe.
> 3. Add component binding operations for stm drm drivers.
personally I'd like number (3) a lot ;-) .
With your approach, at least the component-based variants would
end up with multiple probe cycles, getting clocks etc until at some point
the panel has probed, where in the current way of things, the probe is
done once and we continue bringup once the panel has probed and called
dsi-attach to signal it is present.
Which was actually what Andrzej wished for, when I moved the Rockchip
dsi to the common driver.
Or at least make it configurable via a param to the common dw-dsi probe
function. Especially as I also need the dsi bridge-less when used as a
simple means for the configuring the internal dphy to rx-mode, see [0]
Heiko
[0] https://lore.kernel.org/dri-devel/20210202145632.1263136-1-he...@sntech.de/
> Option 1 is a relatively possible solution as most of the
> mainline drm dsi with bridge drivers have a similar approach
> to their dsi host vs bridge registration.
>
> Signed-off-by: Jagan Teki
> ---
> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 35 +--
> 1 file changed, 17 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index 6b268f9445b3..8a535041f071 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -314,8 +314,6 @@ static int dw_mipi_dsi_host_attach(struct mipi_dsi_host
> *host,
> {
> struct dw_mipi_dsi *dsi = host_to_dsi(host);
> const struct dw_mipi_dsi_plat_data *pdata = dsi->plat_data;
> - struct drm_bridge *bridge;
> - struct drm_panel *panel;
> int ret;
>
> if (device->lanes > dsi->plat_data->max_data_lanes) {
> @@ -329,22 +327,6 @@ static int dw_mipi_dsi_host_attach(struct mipi_dsi_host
> *host,
> dsi->format = device->format;
> dsi->mode_flags = device->mode_flags;
>
> - ret = drm_of_find_panel_or_bridge(host->dev->of_node, 1, 0,
> - , );
> - if (ret)
> - return ret;
> -
> - if (panel) {
> - bridge = drm_panel_bridge_add_typed(panel,
> - DRM_MODE_CONNECTOR_DSI);
> - if (IS_ERR(bridge))
> - return PTR_ERR(bridge);
> - }
> -
> - dsi->panel_bridge = bridge;
> -
> - drm_bridge_add(>bridge);
> -
> if (pdata->host_ops && pdata->host_ops->attach) {
> ret = pdata->host_ops->attach(pdata->priv_data, device);
> if (ret < 0)
> @@ -1105,6 +1087,8 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
> struct device *dev = >dev;
> struct reset_control *apb_rst;
> struct dw_mipi_dsi *dsi;
> + struct drm_bridge *bridge;
> + struct drm_panel *panel;
> int ret;
>
> dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
> @@ -1167,6 +1151,20 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
> dw_mipi_dsi_debugfs_init(dsi);
> pm_runtime_enable(dev);
>
> + ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0,
> + , );
> + if (ret)
> + return ERR_PTR(ret);
> +
> + if (panel) {
> + bridge = drm_panel_bridge_add_typed(panel,
> + DRM_MODE_CONNECTOR_DSI);
> + if (IS_ERR(bridge))
> + return ERR_PTR(-ENODEV);
> + }
> +
> + dsi->panel_bridge = bridge;
> +
> dsi->dsi_host.ops = _mipi_dsi_host_ops;
> dsi->dsi_host.dev = dev;
> ret = mipi_dsi_host_register(>dsi_host);
> @@ -1181,6 +1179,7 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
> #ifdef CONFIG_OF
> dsi->bridge.of_node = pdev->dev.of_node;
> #endif
> + drm_bridge_add(>bridge);
>
> return dsi;
> }
>
Re: [PATCH] usb: dwc2: Fix endpoint direction check in ep_from_windex
Am Montag, 25. Januar 2021, 20:13:24 CET schrieb Heiko Stuebner:
> From: Heiko Stuebner
>
> dwc2_hsotg_process_req_status uses ep_from_windex() to retrieve
> the endpoint for the index provided in the wIndex request param.
>
> In a test-case with a rndis gadget running and sending a malformed
> packet to it like:
> dev.ctrl_transfer(
> 0x82, # bmRequestType
> 0x00, # bRequest
> 0x, # wValue
> 0x0001, # wIndex
> 0x00 # wLength
> )
> it is possible to cause a crash:
>
> [ 217.533022] dwc2 ff30.usb: dwc2_hsotg_process_req_status:
> USB_REQ_GET_STATUS
> [ 217.559003] Unable to handle kernel read from unreadable memory at virtual
> address 0088
> ...
> [ 218.313189] Call trace:
> [ 218.330217] ep_from_windex+0x3c/0x54
> [ 218.348565] usb_gadget_giveback_request+0x10/0x20
> [ 218.368056] dwc2_hsotg_complete_request+0x144/0x184
>
> This happens because ep_from_windex wants to compare the endpoint
> direction even if index_to_ep() didn't return an endpoint due to
> the direction not matching.
>
> The fix is easy insofar that the actual direction check is already
> happening when calling index_to_ep() which will return NULL if there
> is no endpoint for the targeted direction, so the offending check
> can go away completely.
>
> Fixes: c6f5c050e2a7 ("usb: dwc2: gadget: add bi-directional endpoint support")
> Signed-off-by: Heiko Stuebner
> Cc: sta...@vger.kernel.org
superseeded by v3, which includes an appropriate Reported-by tag
and removes an now unused variable (in v2).
Re: [PATCH v2] usb: dwc2: Fix endpoint direction check in ep_from_windex
Am Mittwoch, 27. Januar 2021, 09:50:34 CET schrieb Heiko Stuebner:
> From: Heiko Stuebner
>
> dwc2_hsotg_process_req_status uses ep_from_windex() to retrieve
> the endpoint for the index provided in the wIndex request param.
>
> In a test-case with a rndis gadget running and sending a malformed
> packet to it like:
> dev.ctrl_transfer(
> 0x82, # bmRequestType
> 0x00, # bRequest
> 0x, # wValue
> 0x0001, # wIndex
> 0x00 # wLength
> )
> it is possible to cause a crash:
>
> [ 217.533022] dwc2 ff30.usb: dwc2_hsotg_process_req_status:
> USB_REQ_GET_STATUS
> [ 217.559003] Unable to handle kernel read from unreadable memory at virtual
> address 0088
> ...
> [ 218.313189] Call trace:
> [ 218.330217] ep_from_windex+0x3c/0x54
> [ 218.348565] usb_gadget_giveback_request+0x10/0x20
> [ 218.368056] dwc2_hsotg_complete_request+0x144/0x184
>
> This happens because ep_from_windex wants to compare the endpoint
> direction even if index_to_ep() didn't return an endpoint due to
> the direction not matching.
>
> The fix is easy insofar that the actual direction check is already
> happening when calling index_to_ep() which will return NULL if there
> is no endpoint for the targeted direction, so the offending check
> can go away completely.
>
> Fixes: c6f5c050e2a7 ("usb: dwc2: gadget: add bi-directional endpoint support")
> Signed-off-by: Heiko Stuebner
> Cc: sta...@vger.kernel.org
superseeded by v3, which includes an appropriate Reported-by tag
Re: [PATCH 1/8] dt-binding: watchdog: add more Rockchip compatibles to snps,dw-wdt.yaml
Hi Guenter, Am Dienstag, 26. Januar 2021, 05:55:59 CET schrieb Guenter Roeck: > On 1/25/21 3:40 PM, Heiko Stuebner wrote: > > Am Samstag, 23. Januar 2021, 18:34:01 CET schrieb Guenter Roeck: > >> On Fri, Dec 18, 2020 at 01:05:27PM +0100, Johan Jonker wrote: > >>> The watchdog compatible strings are suppose to be SoC orientated. > >>> In the more recently added Rockchip SoC dtsi files only > >>> the fallback string "snps,dw-wdt" is used, so add the following > >>> compatible strings: > >>> > >>> "rockchip,px30-wdt", "snps,dw-wdt" > >>> "rockchip,rk3228-wdt", "snps,dw-wdt" > >>> "rockchip,rk3308-wdt", "snps,dw-wdt" > >>> "rockchip,rk3328-wdt", "snps,dw-wdt" > >>> "rockchip,rk3399-wdt", "snps,dw-wdt" > >>> "rockchip,rv1108-wdt", "snps,dw-wdt" > >>> > >>> make ARCH=arm dtbs_check > >>> DT_SCHEMA_FILES=Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml > >>> > >>> make ARCH=arm64 dtbs_check > >>> DT_SCHEMA_FILES=Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml > >>> > >>> Signed-off-by: Johan Jonker > >>> Acked-by: Rob Herring > >>> Reviewed-by: Heiko Stuebner > >> > >> Reviewed-by: Guenter Roeck > > > > just to clarify, do you expect me to pick up the dt-binding patch > > with the devicetree patches or do you want to take this individual > > patch through the watchdog tree instead? > > > > You'd have to ask Wim since he takes care of actually sending pull requests. > But didn't you say earlier that you wanted to apply the rest of the series > after this one is applied through the watchdog tree ? Yep that was my intent, though somehow I mistook you as also being a watchdog maintainer, hence the confusion. So I'll wait for Wim to apply this patch and then I'll take the rest of the series. Thanks Heiko > >>> --- > >>> Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml | 6 ++ > >>> 1 file changed, 6 insertions(+) > >>> > >>> diff --git a/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml > >>> b/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml > >>> index f7ee9229c..b58596b18 100644 > >>> --- a/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml > >>> +++ b/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml > >>> @@ -18,10 +18,16 @@ properties: > >>>- const: snps,dw-wdt > >>>- items: > >>>- enum: > >>> + - rockchip,px30-wdt > >>>- rockchip,rk3066-wdt > >>>- rockchip,rk3188-wdt > >>> + - rockchip,rk3228-wdt > >>>- rockchip,rk3288-wdt > >>> + - rockchip,rk3308-wdt > >>> + - rockchip,rk3328-wdt > >>>- rockchip,rk3368-wdt > >>> + - rockchip,rk3399-wdt > >>> + - rockchip,rv1108-wdt > >>>- const: snps,dw-wdt > >>> > >>>reg: > >> > > > > > > > > > >
Re: [PATCH v3 1/4] PCI: rockchip: Make 'ep-gpios' DT property optional
Am Mittwoch, 6. Januar 2021, 14:46:14 CET schrieb Chen-Yu Tsai:
> From: Chen-Yu Tsai
>
> The Rockchip PCIe controller DT binding clearly states that 'ep-gpios' is
> an optional property. And indeed there are boards that don't require it.
>
> Make the driver follow the binding by using devm_gpiod_get_optional()
> instead of devm_gpiod_get().
>
> Fixes: e77f847df54c ("PCI: rockchip: Add Rockchip PCIe controller support")
> Fixes: 956cd99b35a8 ("PCI: rockchip: Separate common code from RC driver")
> Fixes: 964bac9455be ("PCI: rockchip: Split out rockchip_pcie_parse_dt() to
> parse DT")
> Signed-off-by: Chen-Yu Tsai
> ---
> Heiko, I dropped you reviewed-by due to the error message change
>
> Changes since v2:
> - Fix error message for failed GPIO
>
> Changes since v1:
> - Rewrite subject to match existing convention and reference
> 'ep-gpios' DT property instead of the 'ep_gpio' field
> ---
> drivers/pci/controller/pcie-rockchip.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-rockchip.c
> b/drivers/pci/controller/pcie-rockchip.c
> index 904dec0d3a88..90c957e3bc73 100644
> --- a/drivers/pci/controller/pcie-rockchip.c
> +++ b/drivers/pci/controller/pcie-rockchip.c
> @@ -118,9 +118,10 @@ int rockchip_pcie_parse_dt(struct rockchip_pcie
> *rockchip)
> }
>
> if (rockchip->is_rc) {
> - rockchip->ep_gpio = devm_gpiod_get(dev, "ep", GPIOD_OUT_HIGH);
> + rockchip->ep_gpio = devm_gpiod_get_optional(dev, "ep",
> GPIOD_OUT_HIGH);
> if (IS_ERR(rockchip->ep_gpio)) {
> - dev_err(dev, "missing ep-gpios property in node\n");
> + dev_err_probe(dev, PTR_ERR(rockchip->ep_gpio),
> + "failed to get ep GPIO\n");
> return PTR_ERR(rockchip->ep_gpio);
looking at [0] shouldn't that be just
return dev_err_probe(dev, PTR_ERR(.)...);
instead of dev_err_probe + additional return?
Heiko
[0] https://elixir.bootlin.com/linux/latest/source/drivers/base/core.c#L4223
> }
> }
>
Re: (subset) [PATCH 1/3] ARM: dts: rockchip: rename thermal subnodes for rk3288.dtsi
Am Montag, 18. Januar 2021, 13:57:33 CET schrieb Heiko Stuebner:
> On Sun, 17 Jan 2021 16:09:51 +0100, Johan Jonker wrote:
> > A test with the command below gives for example this error:
> > /arch/arm/boot/dts/rk3288-tinker.dt.yaml:
> > thermal-zones: 'cpu_thermal', 'gpu_thermal', 'reserve_thermal'
> > do not match any of the regexes:
> > '^[a-zA-Z][a-zA-Z0-9\\-]{1,12}-thermal$', 'pinctrl-[0-9]+'
> >
> > Rename Rockchip rk3288 thermal subnodes
> > so that it ends with "-thermal"
> >
> > [...]
>
> Applied, thanks!
>
> [2/3] arm64: dts: rockchip: rename thermal subnodes for rk3368.dtsi
> commit: 7c96a5cf680ac733becd454e1f2fd9b258fb
> [3/3] arm64: dts: rockchip: rename thermal subnodes for rk3399.dtsi
> commit: e58061b59787270a57839397e50bb4400b9e2de9
I of course also applied 1/3 in
https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git/commit/?h=v5.12-armsoc/dts32=b840662bd55fb6dc9204585f071518123a87b59d
Just b4 didn't seem to be able to cope with 2 separate fetches
for arm32+arm64.
Heiko
Re: (subset) [PATCH v16 0/8] Add Rockchip NFC drivers for RK3308 and others
Am Montag, 18. Januar 2021, 13:57:36 CET schrieb Heiko Stuebner: > On Thu, 10 Dec 2020 08:21:30 +0800, Yifeng Zhao wrote: > > Rockchp's NFC(Nand Flash Controller) has four versions: V600, V622, V800 and > > V900.This series patch can support all four versions. > > > > > > Changes in v16: > > - Fix some comments about 'ret' variable. > > > > [...] > > Applied, thanks! > > [6/8] arm: dts: rockchip: Add NFC node for RV1108 SoC > commit: 2525f194f9dc07c48b0a12697128357068c2e04b > [7/8] arm: dts: rockchip: Add NFC node for RK2928 and other SoCs > commit: 9c2bfe53b2fc4a8a63311f162e80b27978db6c06 > [8/8] arm: dts: rockchip: Add NFC node for RK3036 SoC > commit: 4cd9a03435bcd20ce6f524e3826fd263951c22fe I of course also applied patches 4-5 (arm32) in - https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git/commit/?h=v5.12-armsoc/dts32=2525f194f9dc07c48b0a12697128357068c2e04b - https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git/commit/?h=v5.12-armsoc/dts32=9c2bfe53b2fc4a8a63311f162e80b27978db6c06 just b4 seemed to hickup with my 2 fetches for arm32+arm64. Heiko
Re: [PATCH v1 1/1] time64.h: Consolidated PSEC_PER_SEC definition
Am Dienstag, 12. Januar 2021, 16:37:09 CET schrieb Andy Shevchenko:
> We have currently three users of the PSEC_PER_SEC each of them defining it
> individually. Instead, move it to time64.h to be available for everyone.
>
> There is a new user coming with the same constant in use. It will also
> make its life easier.
>
> Signed-off-by: Andy Shevchenko
> ---
> drivers/net/ethernet/mscc/ocelot_ptp.c | 2 ++
> drivers/phy/phy-core-mipi-dphy.c | 2 --
> drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c | 8
for the Rockchip-part:
Acked-by: Heiko Stuebner
though not sure if that reordering of other includes should be in there
or separate. Don't have a hard opinion, and will let others decide ;-)
Heiko
> include/soc/mscc/ocelot_ptp.h| 2 --
> include/vdso/time64.h| 1 +
> 5 files changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/ethernet/mscc/ocelot_ptp.c
> b/drivers/net/ethernet/mscc/ocelot_ptp.c
> index a33ab315cc6b..87ad2137ba06 100644
> --- a/drivers/net/ethernet/mscc/ocelot_ptp.c
> +++ b/drivers/net/ethernet/mscc/ocelot_ptp.c
> @@ -4,6 +4,8 @@
> * Copyright (c) 2017 Microsemi Corporation
> * Copyright 2020 NXP
> */
> +#include
> +
> #include
> #include
> #include
> diff --git a/drivers/phy/phy-core-mipi-dphy.c
> b/drivers/phy/phy-core-mipi-dphy.c
> index 14e0551cd319..77fe65367ce5 100644
> --- a/drivers/phy/phy-core-mipi-dphy.c
> +++ b/drivers/phy/phy-core-mipi-dphy.c
> @@ -12,8 +12,6 @@
> #include
> #include
>
> -#define PSEC_PER_SEC 1LL
> -
> /*
> * Minimum D-PHY timings based on MIPI D-PHY specification. Derived
> * from the valid ranges specified in Section 6.9, Table 14, Page 41
> diff --git a/drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c
> b/drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c
> index 8af8c6c5cc02..347dc79a18c1 100644
> --- a/drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c
> +++ b/drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c
> @@ -11,16 +11,16 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> +#include
> #include
> +#include
> +
> #include
> #include
> -#include
> -#include
> -
> -#define PSEC_PER_SEC 1LL
>
> #define UPDATE(x, h, l) (((x) << (l)) & GENMASK((h), (l)))
>
> diff --git a/include/soc/mscc/ocelot_ptp.h b/include/soc/mscc/ocelot_ptp.h
> index 6a7388fa7cc5..ded497d72bdb 100644
> --- a/include/soc/mscc/ocelot_ptp.h
> +++ b/include/soc/mscc/ocelot_ptp.h
> @@ -37,8 +37,6 @@ enum {
>
> #define PTP_CFG_MISC_PTP_EN BIT(2)
>
> -#define PSEC_PER_SEC 1LL
> -
> #define PTP_CFG_CLK_ADJ_CFG_ENA BIT(0)
> #define PTP_CFG_CLK_ADJ_CFG_DIR BIT(1)
>
> diff --git a/include/vdso/time64.h b/include/vdso/time64.h
> index 9d43c3f5e89d..b40cfa2aa33c 100644
> --- a/include/vdso/time64.h
> +++ b/include/vdso/time64.h
> @@ -9,6 +9,7 @@
> #define NSEC_PER_MSEC100L
> #define USEC_PER_SEC 100L
> #define NSEC_PER_SEC 10L
> +#define PSEC_PER_SEC 1LL
> #define FSEC_PER_SEC 1000LL
>
> #endif /* __VDSO_TIME64_H */
>
Re: [PATCH 3/3] arm64: dts: rockchip: rk3328: Add Radxa ROCK Pi E
Am Montag, 11. Januar 2021, 04:27:47 CET schrieb Chen-Yu Tsai:
> On Mon, Jan 11, 2021 at 4:06 AM Heiko Stübner wrote:
> >
> > Hi,
> >
> > Am Sonntag, 10. Januar 2021, 16:37:15 CET schrieb Chen-Yu Tsai:
> > > > > + vcc_sd: sdmmc-regulator {
> > > > > + compatible = "regulator-fixed";
> > > > > + gpio = < RK_PD6 GPIO_ACTIVE_LOW>;
> > > > > + pinctrl-names = "default";
> > > > > + pinctrl-0 = <_pin>;
> > > >
> > > > > + regulator-boot-on;
> > > > > + regulator-name = "vcc_sd";
> > > >
> > > > regulator-name above other regulator properties
> > >
> > > That is actually what I was used to, but some other rockchip dts files
> > > have all the properties sorted alphabetically. So I stuck with what I
> > > saw.
> >
> > I try to keep it alphabetical except for the exceptions :-D .
> >
> > regulator-name is such an exception. Similar to compatibles, the
> > regulator-name is an entry needed to see if you're at the right node,
> > so I really like it being the topmost regulator-foo property - just makes
> > reading easier.
> >
> > (same for the compatible first, then regs, interrupts parts, as well
> > as "status-last")
> >
> > But oftentimes, I just fix the ordering when applying - but seem to have
> > missed this somewhere in those "other Rockchip dts files" ;-) .
>
> I was slightly confused. I looked again and yes regulator-name is always the
> first regulator related property. What's off is that in some cases min/max
> voltage comes before always-on/boot-on, and in others vice versa.
>
> For example in the Rock64 and ROC-RK3328-CC device trees, in the fixed
> regulators, always-on/boot-on come before min/max voltage, while in the
> PMIC the other order is used.
That's likely undecidednes on my part ;-)
There could be an argument for a "name, voltages, flags" sorting, but on
the other hand just keeping it alphabetical with the naming on top
creates less special cases.
Heiko
Re: [PATCH 3/3] arm64: dts: rockchip: rk3328: Add Radxa ROCK Pi E
Hi,
Am Sonntag, 10. Januar 2021, 16:37:15 CET schrieb Chen-Yu Tsai:
> > > + vcc_sd: sdmmc-regulator {
> > > + compatible = "regulator-fixed";
> > > + gpio = < RK_PD6 GPIO_ACTIVE_LOW>;
> > > + pinctrl-names = "default";
> > > + pinctrl-0 = <_pin>;
> >
> > > + regulator-boot-on;
> > > + regulator-name = "vcc_sd";
> >
> > regulator-name above other regulator properties
>
> That is actually what I was used to, but some other rockchip dts files
> have all the properties sorted alphabetically. So I stuck with what I
> saw.
I try to keep it alphabetical except for the exceptions :-D .
regulator-name is such an exception. Similar to compatibles, the
regulator-name is an entry needed to see if you're at the right node,
so I really like it being the topmost regulator-foo property - just makes
reading easier.
(same for the compatible first, then regs, interrupts parts, as well
as "status-last")
But oftentimes, I just fix the ordering when applying - but seem to have
missed this somewhere in those "other Rockchip dts files" ;-) .
> > regulator voltage missing
> > make things as complete as possible
> >
> > from fixed-regulator.yaml:
> >
> > description:
> > Any property defined as part of the core regulator binding, defined in
> > regulator.yaml, can also be used. However a fixed voltage regulator is
> > expected to have the regulator-min-microvolt and regulator-max-microvolt
> > to be the same.
>
> However this is not a real regulator; it is merely an on/off switch.
> I believe in this case it should just pass through the voltage from
> its upstream.
regulator-voltages are not marked required so can stay away if it's just
a dumb switch. I guess it's ok both ways and for individual board-
devicetrees the impact either way is minimal.
> > > + {
> > > + status = "okay";
> > > +
> > > + rk805: pmic@18 {
> > > + compatible = "rockchip,rk805";
> > > + reg = <0x18>;
> > > + interrupt-parent = <>;
> > > + interrupts = <6 IRQ_TYPE_LEVEL_LOW>;
> >
> > > + #clock-cells = <1>;
> >
> > all thing that start with "#" down the list
>
> Is there a proper "preferred" sorting method defined somewhere?
I struggle with that often as well, but normally I'd do #clocks to clocks
with out "#", but really don't have a hard preference here.
especially as just ignoring the "#" would make #address-cells + #size-cells
look strangely sorted ... so more of a common sense thingy.
> > > + eth_phy_int_pin: eth-phy-int-pin {
> > > + rockchip,pins = <1 RK_PD0 RK_FUNC_GPIO
> > > _pull_down>;
> > > + };
> > > +
> > > + eth_phy_reset_pin: eth-phy-reset-pin {
> > > + rockchip,pins = <1 RK_PC2 RK_FUNC_GPIO
> > > _pull_down>;
> > > + };
> > > + };
> > > +
> > > + leds {
> > > + led_pin: led-pin {
> > > + rockchip,pins = <3 RK_PA5 RK_FUNC_GPIO
> > > _pull_none>;
> > > + };
> > > + };
> > > +
> > > + pmic {
> > > + pmic_int_l: pmic-int-l {
> > > + rockchip,pins = <2 RK_PA6 RK_FUNC_GPIO
> > > _pull_up>;
> > > + };
> > > + };
> > > +
> >
> > > + usb3 {
> >
> > usb
> >
> > Last numbers in nodenames are more related to the sort order then to
> > capabillity.
> > ie: mmc0, mmc1
> > All usb pin related things here.
>
> I'd say it is more related to functionality in this case, as in "this group
> is for USB3 related pins". Makes more sense if the board supported both USB2
> and USB3.
I'd agree :-) ... especially as usb controllers on Rockchip boards are not
really numbered and I think we already have precedent for
usb2 -> usb version 2 pins in some other boards ;-)
> > > + cap-sd-highspeed;
> > > + disable-wp;
> > > + pinctrl-names = "default";
> > > + pinctrl-0 = <_clk>, <_cmd>, <_dectn>,
> > > <_bus4>;
> > > + vmmc-supply = <_sd>;
> > > + status = "okay";
> > > +};
> > > +
> >
> > > + {
> > > + vref-supply = <_18>;
> > > + status = "okay";
> > > +};
> >
> > What happened to the recovery key from the schematic?
>
> I believe I originally planned on adding it, but failed to find a proper
> key event for it. Any suggestions?
Most boards seem to use the KEY_VENDOR keycode.
Heiko
Re: [PATCH 1/8] dt-binding: watchdog: add more Rockchip compatibles to snps,dw-wdt.yaml
Am Freitag, 18. Dezember 2020, 13:05:27 CET schrieb Johan Jonker: > The watchdog compatible strings are suppose to be SoC orientated. > In the more recently added Rockchip SoC dtsi files only > the fallback string "snps,dw-wdt" is used, so add the following > compatible strings: > > "rockchip,px30-wdt", "snps,dw-wdt" > "rockchip,rk3228-wdt", "snps,dw-wdt" > "rockchip,rk3308-wdt", "snps,dw-wdt" > "rockchip,rk3328-wdt", "snps,dw-wdt" > "rockchip,rk3399-wdt", "snps,dw-wdt" > "rockchip,rv1108-wdt", "snps,dw-wdt" > > make ARCH=arm dtbs_check > DT_SCHEMA_FILES=Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml > > make ARCH=arm64 dtbs_check > DT_SCHEMA_FILES=Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml > > Signed-off-by: Johan Jonker Reviewed-by: Heiko Stuebner I'd like to pick up the devicetree patches (2-8) once this has landed in the watchdog tree. Thanks Heiko
Re: [RFC PATCH v1 1/4] media: dt-bindings: rockchip-rga: add more rga compatible properties
Am Samstag, 19. Dezember 2020, 12:26:50 CET schrieb Johan Jonker: > Add more rga compatible properties. > > "rockchip,px30-rga", "rockchip,rk3288-rga" > "rockchip,rk3328-rga", "rockchip,rk3288-rga" > "rockchip,rk3368-rga", "rockchip,rk3288-rga" > > make ARCH=arm64 dt_binding_check > DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/rockchip-rga.yaml > > Signed-off-by: Johan Jonker this patch1 should probably go through the media-tree. So hopefully media-people will notice this series? > --- > Documentation/devicetree/bindings/media/rockchip-rga.yaml | 6 +- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/media/rockchip-rga.yaml > b/Documentation/devicetree/bindings/media/rockchip-rga.yaml > index dd645ddcc..a609b63bb 100644 > --- a/Documentation/devicetree/bindings/media/rockchip-rga.yaml > +++ b/Documentation/devicetree/bindings/media/rockchip-rga.yaml > @@ -21,7 +21,11 @@ properties: >- const: rockchip,rk3288-rga >- const: rockchip,rk3399-rga >- items: > - - const: rockchip,rk3228-rga > + - enum: > + - rockchip,px30-rga > + - rockchip,rk3228-rga > + - rockchip,rk3328-rga > + - rockchip,rk3368-rga >- const: rockchip,rk3288-rga > >reg: >
Re: [PATCH 2/2] ARM: dts: rockchip: add extra cpu opp points to rk3288-miqi
Hi Demetris,
Am Freitag, 8. Januar 2021, 16:10:36 CET schrieb Demetris Ierokipides:
> Add extra 1.7GHz and 1.8GHz opp points to the MiQi device-tree to improve
> performance.
>
> Signed-off-by: Demetris Ierokipides
> ---
> arch/arm/boot/dts/rk3288-miqi.dts | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/arch/arm/boot/dts/rk3288-miqi.dts
> b/arch/arm/boot/dts/rk3288-miqi.dts
> index 713f55e143c6..1b48855db6ff 100644
> --- a/arch/arm/boot/dts/rk3288-miqi.dts
> +++ b/arch/arm/boot/dts/rk3288-miqi.dts
> @@ -96,6 +96,18 @@ {
> cpu-supply = <_cpu>;
> };
>
> +_opp_table {
> + opp-170400 {
> + opp-hz = /bits/ 64 <170400>;
> + opp-microvolt = <135>;
> + };
> +
> + opp-18 {
> + opp-hz = /bits/ 64 <18>;
> + opp-microvolt = <135>;
> + };
> +};
> +
sorry, but no .
The OPPs in the mainline kernel match the specifications released by the
soc vendor. Going outside these specs _may_ affect things like stability
on some boards or in general simply the lifetime of the chip itself.
So yes, while boards may generally work with these higher frequencies
I don't think this should be the default coming from the mainline kernel.
If board-owners feel the need to exceed the specs they can still modify
the dts, but we shouldn't force this decision on everyone.
Heiko
Re: [PATCH 5/5] media: hantro: Add support for the Rockchip PX30
Am Freitag, 8. Januar 2021, 11:48:26 CET schrieb Heiko Stübner:
> Am Freitag, 8. Januar 2021, 10:05:16 CET schrieb Paul Kocialkowski:
> > Hi Ezequiel,
> >
> > On Thu 07 Jan 21, 16:08, Ezequiel Garcia wrote:
> > > Happy to see this patch. It was on my TODO list,
> > > but I hadn't had time to bringup my rk3326 device.
> >
> > Same here, I just had an occasion to use it again these days so I jumped
> > on it!
> >
> > > A few comments.
> > >
> > > On Thu, 2021-01-07 at 14:41 +0100, Paul Kocialkowski wrote:
> > > > The PX30 SoC includes both the VDPU2 and VEPU2 blocks which are similar
> > > > to the RK3399 (Hantro G1/H1 with shuffled registers).
> > > >
> > > > Besides taking an extra clock, it also shares an interrupt with the
> > > > IOMMU
> > > > so it's necessary to request the interrupt shared.
> > > >
> > >
> > > Could you clarify on the commit description which iommu device interrupt
> > > line is being shared?
> >
> > Sure! It's IRQ 79 of the GIC that's shared with vopl_mmu.
> > It's not very obvious in the dt commit.
>
> Having looked through the docs again, I think that the vopl_mmu using
> irq 79 is just a mistake:
>
> (1) in general vop and vop-mmu use the same irq (78 in that case)
> (2) Rockchip does seem to have fixed that in their 4.19 tree as well:
> https://github.com/rockchip-linux/kernel/blob/develop-4.19/arch/arm64/boot/dts/rockchip/px30.dtsi#L1598
(3)
https://github.com/rockchip-linux/kernel/commit/391a5c5f96d177896f9fe92ca1c83e00f4352191
;-)
>
> So to me it looks like this doesn't need to be shared and instead
> "simply" the px30 dtsi fixed ;-)
>
>
> Heiko
>
>
> > > > Signed-off-by: Paul Kocialkowski
> > > > ---
> > > > drivers/staging/media/hantro/hantro_drv.c| 5 +++--
> > > > drivers/staging/media/hantro/hantro_hw.h | 1 +
> > > > drivers/staging/media/hantro/rk3399_vpu_hw.c | 21
> > > > 3 files changed, 25 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/staging/media/hantro/hantro_drv.c
> > > > b/drivers/staging/media/hantro/hantro_drv.c
> > > > index e5f200e64993..076a7782b476 100644
> > > > --- a/drivers/staging/media/hantro/hantro_drv.c
> > > > +++ b/drivers/staging/media/hantro/hantro_drv.c
> > > > @@ -472,6 +472,7 @@ static const struct v4l2_file_operations
> > > > hantro_fops = {
> > > >
> > > > static const struct of_device_id of_hantro_match[] = {
> > > > #ifdef CONFIG_VIDEO_HANTRO_ROCKCHIP
> > > > + { .compatible = "rockchip,px30-vpu", .data = _vpu_variant,
> > > > },
> > > > { .compatible = "rockchip,rk3399-vpu", .data =
> > > > _vpu_variant, },
> > > > { .compatible = "rockchip,rk3328-vpu", .data =
> > > > _vpu_variant, },
> > > > { .compatible = "rockchip,rk3288-vpu", .data =
> > > > _vpu_variant, },
> > > > @@ -796,8 +797,8 @@ static int hantro_probe(struct platform_device
> > > > *pdev)
> > > > return -ENXIO;
> > > >
> > > > ret = devm_request_irq(vpu->dev, irq,
> > > > - vpu->variant->irqs[i].handler, 0,
> > > > - dev_name(vpu->dev), vpu);
> > > > + vpu->variant->irqs[i].handler,
> > > > + IRQF_SHARED, dev_name(vpu->dev),
> > > > vpu);
> > >
> > > Maybe this irq flag should be part of vpu->variant? It sounds like an IP
> > > block
> > > integration specific thing.
> >
> > Ah right, I agree that it would be justified. But it would also be simple to
> > just fix the irq handlers and assume this can generally be the case,
> > because it
> > feels like a bit of a detail to justify a flag.
> >
> > Do you think this could be a safe/workable assumption?
> >
> > > Also, you will need a px30-specific interrupt handler now,
> > > since the rk3399 one is not shared-friendly.
> >
> > Yeah I realize I haven't been very careful there and didn't really check
> > that
> > the IOMMU driver is really safe to handle shared interrupts either. I'll
> > take
> > a look a that when crafting v2.
> &
Re: [PATCH 5/5] media: hantro: Add support for the Rockchip PX30
Am Freitag, 8. Januar 2021, 10:05:16 CET schrieb Paul Kocialkowski:
> Hi Ezequiel,
>
> On Thu 07 Jan 21, 16:08, Ezequiel Garcia wrote:
> > Happy to see this patch. It was on my TODO list,
> > but I hadn't had time to bringup my rk3326 device.
>
> Same here, I just had an occasion to use it again these days so I jumped
> on it!
>
> > A few comments.
> >
> > On Thu, 2021-01-07 at 14:41 +0100, Paul Kocialkowski wrote:
> > > The PX30 SoC includes both the VDPU2 and VEPU2 blocks which are similar
> > > to the RK3399 (Hantro G1/H1 with shuffled registers).
> > >
> > > Besides taking an extra clock, it also shares an interrupt with the IOMMU
> > > so it's necessary to request the interrupt shared.
> > >
> >
> > Could you clarify on the commit description which iommu device interrupt
> > line is being shared?
>
> Sure! It's IRQ 79 of the GIC that's shared with vopl_mmu.
> It's not very obvious in the dt commit.
Having looked through the docs again, I think that the vopl_mmu using
irq 79 is just a mistake:
(1) in general vop and vop-mmu use the same irq (78 in that case)
(2) Rockchip does seem to have fixed that in their 4.19 tree as well:
https://github.com/rockchip-linux/kernel/blob/develop-4.19/arch/arm64/boot/dts/rockchip/px30.dtsi#L1598
So to me it looks like this doesn't need to be shared and instead
"simply" the px30 dtsi fixed ;-)
Heiko
> > > Signed-off-by: Paul Kocialkowski
> > > ---
> > > drivers/staging/media/hantro/hantro_drv.c| 5 +++--
> > > drivers/staging/media/hantro/hantro_hw.h | 1 +
> > > drivers/staging/media/hantro/rk3399_vpu_hw.c | 21
> > > 3 files changed, 25 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/staging/media/hantro/hantro_drv.c
> > > b/drivers/staging/media/hantro/hantro_drv.c
> > > index e5f200e64993..076a7782b476 100644
> > > --- a/drivers/staging/media/hantro/hantro_drv.c
> > > +++ b/drivers/staging/media/hantro/hantro_drv.c
> > > @@ -472,6 +472,7 @@ static const struct v4l2_file_operations hantro_fops
> > > = {
> > >
> > > static const struct of_device_id of_hantro_match[] = {
> > > #ifdef CONFIG_VIDEO_HANTRO_ROCKCHIP
> > > + { .compatible = "rockchip,px30-vpu", .data = _vpu_variant, },
> > > { .compatible = "rockchip,rk3399-vpu", .data =
> > > _vpu_variant, },
> > > { .compatible = "rockchip,rk3328-vpu", .data =
> > > _vpu_variant, },
> > > { .compatible = "rockchip,rk3288-vpu", .data =
> > > _vpu_variant, },
> > > @@ -796,8 +797,8 @@ static int hantro_probe(struct platform_device *pdev)
> > > return -ENXIO;
> > >
> > > ret = devm_request_irq(vpu->dev, irq,
> > > - vpu->variant->irqs[i].handler, 0,
> > > - dev_name(vpu->dev), vpu);
> > > + vpu->variant->irqs[i].handler,
> > > + IRQF_SHARED, dev_name(vpu->dev),
> > > vpu);
> >
> > Maybe this irq flag should be part of vpu->variant? It sounds like an IP
> > block
> > integration specific thing.
>
> Ah right, I agree that it would be justified. But it would also be simple to
> just fix the irq handlers and assume this can generally be the case, because
> it
> feels like a bit of a detail to justify a flag.
>
> Do you think this could be a safe/workable assumption?
>
> > Also, you will need a px30-specific interrupt handler now,
> > since the rk3399 one is not shared-friendly.
>
> Yeah I realize I haven't been very careful there and didn't really check that
> the IOMMU driver is really safe to handle shared interrupts either. I'll take
> a look a that when crafting v2.
>
> > > if (ret) {
> > > dev_err(vpu->dev, "Could not request %s IRQ.\n",
> > > irq_name);
> > > diff --git a/drivers/staging/media/hantro/hantro_hw.h
> > > b/drivers/staging/media/hantro/hantro_hw.h
> > > index 34c9e4649a25..07f516fd7a2e 100644
> > > --- a/drivers/staging/media/hantro/hantro_hw.h
> > > +++ b/drivers/staging/media/hantro/hantro_hw.h
> > > @@ -148,6 +148,7 @@ enum hantro_enc_fmt {
> > > RK3288_VPU_ENC_FMT_UYVY422 = 3,
> > > };
> > >
> > > +extern const struct hantro_variant px30_vpu_variant;
> > > extern const struct hantro_variant rk3399_vpu_variant;
> > > extern const struct hantro_variant rk3328_vpu_variant;
> > > extern const struct hantro_variant rk3288_vpu_variant;
> > > diff --git a/drivers/staging/media/hantro/rk3399_vpu_hw.c
> > > b/drivers/staging/media/hantro/rk3399_vpu_hw.c
> > > index 7a7962cf771e..4112f98baa60 100644
> > > --- a/drivers/staging/media/hantro/rk3399_vpu_hw.c
> > > +++ b/drivers/staging/media/hantro/rk3399_vpu_hw.c
> >
> > Perhaps it's time to rename this to rockchip_vpu_hw.c,
> > and merge rk3288 and rk3399? It's a nitpick, though.
>
> Haha, I was thinking the exact same thing but wasn't sure it would be welcome!
>
> I was thinking
Re: linux-next: Signed-off-by missing for commit in the rockchip tree
Hi Stephen,
Am Montag, 30. November 2020, 22:21:07 CET schrieb Stephen Rothwell:
> Hi all,
>
> Commit
>
> f166ed782080 ("arm64: defconfig: Enable RTC_DRV_HYM8563")
>
> is missing a Signed-off-by from its committer.
thanks for catching this.
I've addy the Signed-off-by and re-created the for-next branch.
Thanks
Heiko
Re: (subset) [PATCH 1/2] arm64: defconfig: Enable RTC_DRV_HYM8563
Am Montag, 30. November 2020, 14:50:21 CET schrieb Heiko Stuebner: > On Fri, 23 Oct 2020 23:48:13 +0530, Jagan Teki wrote: > > RTC HYM8563 used in the ARM64 Rockchip SoC's SDIO power > > sequence enablement. > > > > Enable it as module. > > Applied both patches, thanks! I've also split the hym8563 addition out into a separate patch in front.
Re: [PATCH v3] arm64: dts: rockchip: add SPDIF node for rk3399-rockpro64
Am Dienstag, 6. Oktober 2020, 11:51:58 CET schrieb Johan Jonker:
> Hi Katsuhiro, Heiko,
>
> Question for the maintainer:
> Should we add a SPDIF node if the connector is not physical on a board,
> only a header?
I think so ... the connector always is just like n-pins on a board,
only "sometimes" someone solders a fancy jack to it ;-)
Heiko
>
> Thanks Katsuhiro for the "aplay -l" screen print.
>
> List of PLAYBACK Hardware Devices
> card 0: hdmisound [hdmi-sound], device 0: ff8a.i2s-i2s-hifi
> i2s-hifi-0 [ff8a.i2s-i2s-hifi i2s-hifi-0]
> Subdevices: 1/1
> Subdevice #0: subdevice #0
> card 1: rockchiprk3399 [rockchip,rk3399], device 0: ff89.i2s-ES8316
> HiFi ES8316 HiFi-0 [ff89.i2s-ES8316 HiFi ES8316 HiFi-0]
> Subdevices: 1/1
> Subdevice #0: subdevice #0
> card 2: rockchiprk339_1 [rockchip,rk3399], device 0:
> ff87.spdif-dit-hifi dit-hifi-0 [ff87.spdif-dit-hifi dit-hifi-0]
> Subdevices: 1/1
> Subdevice #0: subdevice #0
>
>
> On 10/5/20 4:03 PM, Katsuhiro Suzuki wrote:
> > This patch adds 'disabled' SPDIF sound node and related settings
> > of SPDIF for rk3399-rockpro64.
> >
> > RockPro64 has output pins for SPDIF Tx. But RK3399 does not have
> > enough DMA channel for enabling SPDIF tx. Current settings are:
> >
> > - I2S0 (Req num 0, 1): Enabled : Output to 40pins header CON40
> > - I2S1 (Req num 2, 3): Enabled : Output to ES8316 on board
> > - I2S2 (Req num 4, 5): Enabled : Output to internal HDMI core
> > - SPDIF Tx (Req num 7) : Disabled: Output to connector J10
> >
> > If users want to enable ALL sound I/Os, we need 7 DMA channels for
> > it. But unfortunately, RK3399 has only 6 DMA channels. So users have
> > to choose from the following:
> >
> > - Disable one of I2S (Ex. I2S0) and enable SPDIF tx
> > - Keep enable I2S0/1/2 and disable SPDIF tx
> >
> > Signed-off-by: Katsuhiro Suzuki
> >
> > ---
> >
> > Changes in v3:
> > - Refine commit description why adding disabled node
> >
> > Changes in v2:
> > - Remove redundant status property
> > ---
> > .../boot/dts/rockchip/rk3399-rockpro64.dtsi | 27 +++
> > 1 file changed, 27 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
> > b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
> > index 6e553ff47534..58097245994a 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
> > @@ -76,6 +76,23 @@ sound {
> > dais = <_p0>;
> > };
> >
>
>
> hdmi_sound: hdmi-sound {
> compatible = "simple-audio-card";
> simple-audio-card,name = "hdmi-sound";
>
> Maybe rename to "HDMI"?
>
> [..]
> };
>
> sound {
> compatible = "audio-graph-card";
> label = "rockchip,rk3399";
>
> Maybe change this to "ES8316" to prevent confusion?
>
> dais = <_p0>;
> };
>
>
> > + sound-dit {
> > + compatible = "audio-graph-card"
> > + label = "rockchip,rk3399";
>
> This would be the second sound card with the same label.
> It seems that aplay/linux? adds "-1" to it and removes the comma, so we get:
>
> hdmisound
> rockchiprk3399
> rockchiprk339_1
>
> Shouldn't we label it with something that reflect the function/output.
> Shouldn't we standardize to SPDIF, HDMI and Analog similar to rk3318/rk3328?
> Make a shorter label without spaces or special chars, so that chars
> don't get removed?
>
> Proposal:
>
> HDMI
> ES8316
> SPDIF
>
> > + dais = <_p0>;
>
> Maybe disable too?
>
> The "sound-dit" node is standard enabled and will start some process
> with a dia in a node that is disabled.
>
>
> > + };
> > +
> > + spdif-dit {
> > + compatible = "linux,spdif-dit";
> > + #sound-dai-cells = <0>;
>
> Maybe disable too?
>
> > +
> > + port {
> > + dit_p0_0: endpoint {
>
> > + remote-endpoint = <_p0_0>;
>
> This also points to something that's disabled.
>
> > + };
> > + };
> > + };
> > +
> > vcc12v_dcin: vcc12v-dcin {
> > compatible = "regulator-fixed";
> > regulator-name = "vcc12v_dcin";
> > @@ -698,6 +715,16 @@ {
> > status = "okay";
> > };
> >
> > + {
> > + pinctrl-0 = <_bus_1>;
>
> This node is disabled.
>
> > +
> > + spdif_p0: port {
> > + spdif_p0_0: endpoint {
> > + remote-endpoint = <_p0_0>;
> > + };
> > + };
> > +};
> > +
> > {
> > status = "okay";
> >
> >
>
>
Re: [PATCH] drm/rockchip: dw_hdmi: fix incorrect clock in vpll clock error message
Am Samstag, 24. Oktober 2020, 05:53:21 CET schrieb Jonathan Liu: > Error message incorrectly refers to grf clock instead of vpll clock. > > Signed-off-by: Jonathan Liu applied to drm-misc-next Thanks Heiko
Re: [PATCH] phy/rockchip: Make PHY_ROCKCHIP_INNO_HDMI depend on HAS_IOMEM to fix build error
Am Dienstag, 24. November 2020, 12:11:27 CET schrieb Tiezhu Yang: > devm_ioremap_resource() will be not built in lib/devres.c if > CONFIG_HAS_IOMEM is not set, and then there exists a build > error about undefined reference to "devm_ioremap_resource" > in the file phy-rockchip-inno-hdmi.c under COMPILE_TEST and > CONFIG_PHY_ROCKCHIP_INNO_HDMI, make PHY_ROCKCHIP_INNO_HDMI > depend on HAS_IOMEM to fix it. > > Reported-by: kernel test robot > Signed-off-by: Tiezhu Yang Reviewed-by: Heiko Stuebner > --- > drivers/phy/rockchip/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/phy/rockchip/Kconfig b/drivers/phy/rockchip/Kconfig > index c2f22f9..159285f 100644 > --- a/drivers/phy/rockchip/Kconfig > +++ b/drivers/phy/rockchip/Kconfig > @@ -32,6 +32,7 @@ config PHY_ROCKCHIP_INNO_HDMI > tristate "Rockchip INNO HDMI PHY Driver" > depends on (ARCH_ROCKCHIP || COMPILE_TEST) && OF > depends on COMMON_CLK > + depends on HAS_IOMEM > select GENERIC_PHY > help > Enable this to support the Rockchip Innosilicon HDMI PHY. >
Re: [PATCH v2 1/4] PCI: rockchip: Make 'ep-gpios' DT property optional
Am Mittwoch, 18. November 2020, 08:17:21 CET schrieb Chen-Yu Tsai:
> From: Chen-Yu Tsai
>
> The Rockchip PCIe controller DT binding clearly states that 'ep-gpios' is
> an optional property. And indeed there are boards that don't require it.
>
> Make the driver follow the binding by using devm_gpiod_get_optional()
> instead of devm_gpiod_get().
>
> Fixes: e77f847df54c ("PCI: rockchip: Add Rockchip PCIe controller support")
> Fixes: 956cd99b35a8 ("PCI: rockchip: Separate common code from RC driver")
> Fixes: 964bac9455be ("PCI: rockchip: Split out rockchip_pcie_parse_dt() to
> parse DT")
> Signed-off-by: Chen-Yu Tsai
Reviewed-by: Heiko Stuebner ---
> Changes since v1:
>
> - Rewrite subject to match existing convention and reference
> 'ep-gpios' DT property instead of the 'ep_gpio' field
> ---
> drivers/pci/controller/pcie-rockchip.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/pcie-rockchip.c
> b/drivers/pci/controller/pcie-rockchip.c
> index 904dec0d3a88..c95950e9004f 100644
> --- a/drivers/pci/controller/pcie-rockchip.c
> +++ b/drivers/pci/controller/pcie-rockchip.c
> @@ -118,7 +118,7 @@ int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip)
> }
>
> if (rockchip->is_rc) {
> - rockchip->ep_gpio = devm_gpiod_get(dev, "ep", GPIOD_OUT_HIGH);
> + rockchip->ep_gpio = devm_gpiod_get_optional(dev, "ep",
> GPIOD_OUT_HIGH);
> if (IS_ERR(rockchip->ep_gpio)) {
> dev_err(dev, "missing ep-gpios property in node\n");
> return PTR_ERR(rockchip->ep_gpio);
>
Re: [PATCH 05/25] soc: rockchip: io-domain: Remove incorrect and incomplete comment header
Am Donnerstag, 12. November 2020, 14:28:28 CET schrieb Lee Jones:
> On Thu, 12 Nov 2020, Heiko Stübner wrote:
>
> > Am Donnerstag, 12. November 2020, 14:22:24 CET schrieb Lee Jones:
> > > On Thu, 12 Nov 2020, Heiko Stübner wrote:
> > >
> > > > Am Donnerstag, 12. November 2020, 11:33:44 CET schrieb Lee Jones:
> > > > > On Tue, 03 Nov 2020, Lee Jones wrote:
> > > > >
> > > > > > Fixes the following W=1 kernel build warning(s):
> > > > > >
> > > > > > drivers/soc/rockchip/io-domain.c:57: warning: Cannot understand *
> > > > > > @supplies: voltage settings matching the register bits.
> > > > > >
> > > > > > Cc: Heiko Stuebner
> > > > > > Cc: Liam Girdwood
> > > > > > Cc: Mark Brown
> > > > > > Cc: "Rafael J. Wysocki"
> > > > > > Cc: Doug Anderson
> > > > > > Cc: linux-rockc...@lists.infradead.org
> > > > > > Signed-off-by: Lee Jones
> > > > > > ---
> > > > > > drivers/soc/rockchip/io-domain.c | 3 ---
> > > > > > 1 file changed, 3 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/soc/rockchip/io-domain.c
> > > > > > b/drivers/soc/rockchip/io-domain.c
> > > > > > index eece97f97ef8f..d13d2d497720b 100644
> > > > > > --- a/drivers/soc/rockchip/io-domain.c
> > > > > > +++ b/drivers/soc/rockchip/io-domain.c
> > > > > > @@ -53,9 +53,6 @@
> > > > > >
> > > > > > struct rockchip_iodomain;
> > > > > >
> > > > > > -/**
> > > > > > - * @supplies: voltage settings matching the register bits.
> > > > > > - */
> > > > > > struct rockchip_iodomain_soc_data {
> > > > > > int grf_offset;
> > > > > > const char *supply_names[MAX_SUPPLIES];
> > > > >
> > > > > Any idea who will pick this up?
> > > >
> > > > me :-)
> > >
> > > Well, that's certainly a start. :)
> > >
> > > What are your plans?
> >
> > the usual, my rockchip-tree -> armsoc driver branch -> torvalds -> 5.11 ;-)
>
> Sorry, the ambiguity was my fault.
>
> When do you plan on hoovering it up?
sorry should've written that directly ... I already did this "morning":
http://lore.kernel.org/r/160517975455.81506.16289432612279089945.b4...@sntech.de
Heiko
Re: [PATCH 05/25] soc: rockchip: io-domain: Remove incorrect and incomplete comment header
Am Donnerstag, 12. November 2020, 14:22:24 CET schrieb Lee Jones:
> On Thu, 12 Nov 2020, Heiko Stübner wrote:
>
> > Am Donnerstag, 12. November 2020, 11:33:44 CET schrieb Lee Jones:
> > > On Tue, 03 Nov 2020, Lee Jones wrote:
> > >
> > > > Fixes the following W=1 kernel build warning(s):
> > > >
> > > > drivers/soc/rockchip/io-domain.c:57: warning: Cannot understand *
> > > > @supplies: voltage settings matching the register bits.
> > > >
> > > > Cc: Heiko Stuebner
> > > > Cc: Liam Girdwood
> > > > Cc: Mark Brown
> > > > Cc: "Rafael J. Wysocki"
> > > > Cc: Doug Anderson
> > > > Cc: linux-rockc...@lists.infradead.org
> > > > Signed-off-by: Lee Jones
> > > > ---
> > > > drivers/soc/rockchip/io-domain.c | 3 ---
> > > > 1 file changed, 3 deletions(-)
> > > >
> > > > diff --git a/drivers/soc/rockchip/io-domain.c
> > > > b/drivers/soc/rockchip/io-domain.c
> > > > index eece97f97ef8f..d13d2d497720b 100644
> > > > --- a/drivers/soc/rockchip/io-domain.c
> > > > +++ b/drivers/soc/rockchip/io-domain.c
> > > > @@ -53,9 +53,6 @@
> > > >
> > > > struct rockchip_iodomain;
> > > >
> > > > -/**
> > > > - * @supplies: voltage settings matching the register bits.
> > > > - */
> > > > struct rockchip_iodomain_soc_data {
> > > > int grf_offset;
> > > > const char *supply_names[MAX_SUPPLIES];
> > >
> > > Any idea who will pick this up?
> >
> > me :-)
>
> Well, that's certainly a start. :)
>
> What are your plans?
the usual, my rockchip-tree -> armsoc driver branch -> torvalds -> 5.11 ;-)
Re: [PATCH 05/25] soc: rockchip: io-domain: Remove incorrect and incomplete comment header
Am Donnerstag, 12. November 2020, 11:33:44 CET schrieb Lee Jones:
> On Tue, 03 Nov 2020, Lee Jones wrote:
>
> > Fixes the following W=1 kernel build warning(s):
> >
> > drivers/soc/rockchip/io-domain.c:57: warning: Cannot understand *
> > @supplies: voltage settings matching the register bits.
> >
> > Cc: Heiko Stuebner
> > Cc: Liam Girdwood
> > Cc: Mark Brown
> > Cc: "Rafael J. Wysocki"
> > Cc: Doug Anderson
> > Cc: linux-rockc...@lists.infradead.org
> > Signed-off-by: Lee Jones
> > ---
> > drivers/soc/rockchip/io-domain.c | 3 ---
> > 1 file changed, 3 deletions(-)
> >
> > diff --git a/drivers/soc/rockchip/io-domain.c
> > b/drivers/soc/rockchip/io-domain.c
> > index eece97f97ef8f..d13d2d497720b 100644
> > --- a/drivers/soc/rockchip/io-domain.c
> > +++ b/drivers/soc/rockchip/io-domain.c
> > @@ -53,9 +53,6 @@
> >
> > struct rockchip_iodomain;
> >
> > -/**
> > - * @supplies: voltage settings matching the register bits.
> > - */
> > struct rockchip_iodomain_soc_data {
> > int grf_offset;
> > const char *supply_names[MAX_SUPPLIES];
>
> Any idea who will pick this up?
me :-)
Re: [PATCH v4 0/7] arm64: dts: rockchip: Add Engicam PX30.Core
Am Mittwoch, 4. November 2020, 20:54:40 CET schrieb Jagan Teki: > On Thu, Oct 22, 2020 at 12:27 AM Jagan Teki > wrote: > > > > Hi Heiko, > > > > On Tue, Sep 29, 2020 at 2:02 PM Jagan Teki > > wrote: > > > > > > PX30.Core is an EDIMM SOM based on Rockchip PX30 from Engicam. > > > > > > PX30.Core needs to mount on top of Engicam baseboards for creating > > > complete platform boards. > > > > > > Possible baseboards are, > > > - EDIMM2.2 Starter Kit > > > - C.TOUCH 2.0 Carrier Board > > > > > > Changes for v4: > > > - collect Rob A-b > > > Changes for v3: > > > - resolved Johan comments about sorting node properties > > > - add copyright to Amarula Solutions > > > - update px30 dtsi author > > > Changes for v2: > > > - include C.TOUCH 2.0 carrier board > > > - skip 10" OF LCD as it requires separate dts with panel support. > > > > > > Note: These baseboards can be used for i.MX8 SOM's as well. So having > > > baseboard on respective SoC seems to be easy rather than making it > > > common across all. > > > > > > Any inputs? > > > Jagan. > > > > > > Jagan Teki (6): > > > dt-bindings: arm: rockchip: Add Engicam PX30.Core EDIMM2.2 Starter Kit > > > arm64: dts: rockchip: px30: Add Engicam EDIMM2.2 Starter Kit > > > arm64: dts: rockchip: Add Engicam PX30.Core EDIMM2.2 Starter Kit > > > dt-bindings: arm: rockchip: Add Engicam PX30.Core C.TOUCH 2.0 > > > arm64: dts: rockchip: px30: Add Engicam C.TOUCH 2.0 > > > arm64: dts: rockchip: Add Engicam PX30.Core C.TOUCH 2.0 > > > > > > Michael Trimarchi (1): > > > arm64: dts: rockchip: Add Engicam PX30.Core SOM > > > > > > .../devicetree/bindings/arm/rockchip.yaml | 12 + > > > arch/arm64/boot/dts/rockchip/Makefile | 2 + > > > .../dts/rockchip/px30-engicam-common.dtsi | 39 +++ > > > .../dts/rockchip/px30-engicam-ctouch2.dtsi| 8 + > > > .../dts/rockchip/px30-engicam-edimm2.2.dtsi | 7 + > > > .../dts/rockchip/px30-px30-core-ctouch2.dts | 22 ++ > > > .../dts/rockchip/px30-px30-core-edimm2.2.dts | 21 ++ > > > .../boot/dts/rockchip/px30-px30-core.dtsi | 232 ++ > > > 8 files changed, 343 insertions(+) > > > create mode 100644 arch/arm64/boot/dts/rockchip/px30-engicam-common.dtsi > > > create mode 100644 arch/arm64/boot/dts/rockchip/px30-engicam-ctouch2.dtsi > > > create mode 100644 > > > arch/arm64/boot/dts/rockchip/px30-engicam-edimm2.2.dtsi > > > create mode 100644 > > > arch/arm64/boot/dts/rockchip/px30-px30-core-ctouch2.dts > > > create mode 100644 > > > arch/arm64/boot/dts/rockchip/px30-px30-core-edimm2.2.dts > > > create mode 100644 arch/arm64/boot/dts/rockchip/px30-px30-core.dtsi > > > > Any further comments? > > Gentle ping. on my list to untangle :-) Heiko
Re: [PATCH] arm64: dts: rockchip: Assign a fixed index to mmc devices on rk3399-roc-pc boards.
Am Mittwoch, 4. November 2020, 16:42:01 CET schrieb Doug Anderson:
> Hi,
>
> On Wed, Nov 4, 2020 at 2:51 AM Heiko Stübner wrote:
> >
> > Hi Markus,
> >
> > Am Mittwoch, 4. November 2020, 10:49:45 CET schrieb Markus Reichl:
> > > Recently introduced async probe on mmc devices can shuffle block IDs.
> > > Pin them to fixed values to ease booting in evironments where UUIDs
> > > are not practical. Use newly introduced aliases for mmcblk devices from
> > > [1].
> > >
> > > [1]
> > > https://patchwork.kernel.org/patch/11747669/
> > >
> > > Signed-off-by: Markus Reichl
> > > ---
> > > arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi | 5 +
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
> > > b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
> > > index e7a459fa4322..bc9482b59428 100644
> > > --- a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
> > > +++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
> > > @@ -13,6 +13,11 @@ / {
> > > model = "Firefly ROC-RK3399-PC Board";
> > > compatible = "firefly,roc-rk3399-pc", "rockchip,rk3399";
> > >
> > > + aliases {
> > > + mmc0 =
> > > + mmc1 =
> > > + };
> > > +
> >
> > Any reason for this odering?
> >
> > I.e. some previous incarnations had it ordered as (emmc, mmc, sdio).
> > This is also true for the ChromeOS out-of-tree usage of those, the
> > rk3399 dts in the chromeos-4.4 tree also orders this as sdhci, sdmmc, sdio.
> >
> > And I guess a further question would be when we're doing arbitary orderings
> > anyway, why is this not in rk3399.dtsi ;-) ?
>
> Though I personally like the idea of eMMC, which is typically
> built-in, as being the "0" number, I'm personally happy with any
> numbering scheme that's consistent. Ordering them by base address is
> OK w/ me and seems less controversial. That seems like it could go in
> rk3399.dtsi and then if a particular board wanted a different order
> they could override it in their board file.
Yep that sounds sensible and ordering by base address at least is one
"simple" type of order without too much explanation needed.
So I guess we'd get a sdio + sdmmc + sdhci ordering
@Markus: if nobody else complains, can you do a "simple" rk3399.dtsi
change with that please?
> The downside of putting
> in rk3399 is that boards that don't have all SD/MMC interfaces enabled
> would definitely get a new number compared to old kernels, but
> hopefully this is the last time?
With that new asynchronous mmc-probe-thingy in 5.10 that "caused" this,
it sounds like everything gets a new number anyway ;-) .
Heiko
Re: [PATCH] arm64: dts: rockchip: Assign a fixed index to mmc devices on rk3399-roc-pc boards.
Hi Markus,
Am Mittwoch, 4. November 2020, 10:49:45 CET schrieb Markus Reichl:
> Recently introduced async probe on mmc devices can shuffle block IDs.
> Pin them to fixed values to ease booting in evironments where UUIDs
> are not practical. Use newly introduced aliases for mmcblk devices from [1].
>
> [1]
> https://patchwork.kernel.org/patch/11747669/
>
> Signed-off-by: Markus Reichl
> ---
> arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
> b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
> index e7a459fa4322..bc9482b59428 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
> @@ -13,6 +13,11 @@ / {
> model = "Firefly ROC-RK3399-PC Board";
> compatible = "firefly,roc-rk3399-pc", "rockchip,rk3399";
>
> + aliases {
> + mmc0 =
> + mmc1 =
> + };
> +
Any reason for this odering?
I.e. some previous incarnations had it ordered as (emmc, mmc, sdio).
This is also true for the ChromeOS out-of-tree usage of those, the
rk3399 dts in the chromeos-4.4 tree also orders this as sdhci, sdmmc, sdio.
And I guess a further question would be when we're doing arbitary orderings
anyway, why is this not in rk3399.dtsi ;-) ?
Heiko
Re: [PATCH v2 1/2] ARM: dts: rk3188: correct interrupt flags
Hi, Am Freitag, 2. Oktober 2020, 18:11:28 CEST schrieb Krzysztof Kozlowski: > On Thu, Sep 17, 2020 at 08:52:10PM +0200, Krzysztof Kozlowski wrote: > > GPIO_ACTIVE_x flags are not correct in the context of interrupt flags. > > These are simple defines so they could be used in DTS but they will not > > have the same meaning: > > 1. GPIO_ACTIVE_HIGH = 0 = IRQ_TYPE_NONE > > 2. GPIO_ACTIVE_LOW = 1 = IRQ_TYPE_EDGE_RISING > > > > Correct the interrupt flags without affecting the code: > > ACTIVE_HIGH => IRQ_TYPE_NONE > > > > Signed-off-by: Krzysztof Kozlowski > > > > --- > > > > Not tested on HW. > > > > Changes since v1: > > 1. Correct title > > --- > > arch/arm/boot/dts/rk3188-bqedison2qc.dts | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > Hi, > > Any comments/review/testing from Heiko or other Rockchip folks? Shall I > cc here someone? I'm actually wondering about this ... I somehow remember writing a response, but don't see it in my history - so it might have gotten lost before I actually sent it. I think the biggest issue I have is that none of that is tested on any hardware and looking at other brcm wifi drivers in the kernel, the interrupt polarity seems to be all over the place, some set it high, some low and I even have seen edge triggers. As all changes are in regard to (copied) brcm wifi node, it would be really interesting to actually know what trigger is the right one. I've Cc'ed Jagan who I think has worked on an affected board, maybe he can check which trigger is correct. Heiko
Re: [PATCH v3 0/3] rockchip-pinctrl fixes
Hi Jianqun, Am Dienstag, 13. Oktober 2020, 08:37:28 CEST schrieb Jianqun Xu: > These patches are required by GKI. > > Jianqun Xu (3): > pinctrl: rockchip: make driver be tristate module > pinctrl: rockchip: enable gpio pclk for rockchip_gpio_to_irq > pinctrl: rockchip: create irq mapping in gpio_to_irq looks good to go as fixes. What happenend to the "populate platform device for rockchip gpio" patch though - more out of cursiosity. Heiko
Re: [PATCH 2/4] power: avs: rockchip-io: Move the driver to the rockchip specific drivers
Am Dienstag, 6. Oktober 2020, 18:05:14 CEST schrieb Ulf Hansson: > The avs drivers are all SoC specific drivers that doesn't share any code. > Instead they are located in a directory, mostly to keep similar > functionality together. From a maintenance point of view, it makes better > sense to collect SoC specific drivers like these, into the SoC specific > directories. > > Therefore, let's move the rockchip-io driver to the rockchip directory. > > Cc: Heiko Stuebner > Cc: linux-rockc...@lists.infradead.org > Signed-off-by: Ulf Hansson Acked-by: Heiko Stuebner
Re: [PATCH] clk: rockchip: Initialize hw to error to avoid undefined behavior
Am Donnerstag, 24. September 2020, 02:44:41 CEST schrieb Stephen Boyd:
> We can get down to this return value from ERR_CAST() without
> initializing hw. Set it to -ENOMEM so that we always return something
> sane.
>
> Fixes the following smatch warning:
>
> drivers/clk/rockchip/clk-half-divider.c:228 rockchip_clk_register_halfdiv()
> error: uninitialized symbol 'hw'.
> drivers/clk/rockchip/clk-half-divider.c:228 rockchip_clk_register_halfdiv()
> warn: passing zero to 'ERR_CAST'
>
> Cc: Elaine Zhang
> Cc: Heiko Stuebner
> Fixes: 956060a52795 ("clk: rockchip: add support for half divider")
> Signed-off-by: Stephen Boyd
Reviewed-by: Heiko Stuebner
> ---
> drivers/clk/rockchip/clk-half-divider.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clk/rockchip/clk-half-divider.c
> b/drivers/clk/rockchip/clk-half-divider.c
> index e97fd3dfbae7..ccd5c270c213 100644
> --- a/drivers/clk/rockchip/clk-half-divider.c
> +++ b/drivers/clk/rockchip/clk-half-divider.c
> @@ -166,7 +166,7 @@ struct clk *rockchip_clk_register_halfdiv(const char
> *name,
> unsigned long flags,
> spinlock_t *lock)
> {
> - struct clk_hw *hw;
> + struct clk_hw *hw = ERR_PTR(-ENOMEM);
> struct clk_mux *mux = NULL;
> struct clk_gate *gate = NULL;
> struct clk_divider *div = NULL;
>
> base-commit: ca52a47af60f791b08a540a8e14d8f5751ee63e9
>
Re: [PATCH] drm/rockchip: skip probed failed device
Am Mittwoch, 23. September 2020, 13:05:26 CEST schrieb Robin Murphy:
> On 2020-09-23 07:59, Jian-Hong Pan wrote:
> > The cdn-dp sub driver probes the device failed on PINEBOOK Pro.
> >
> > kernel: cdn-dp fec0.dp: [drm:cdn_dp_probe [rockchipdrm]] *ERROR*
> > missing extcon or phy
> > kernel: cdn-dp: probe of fec0.dp failed with error -22
>
> Wouldn't it make more sense to simply not enable the DisplayPort node in
> the upstream DT, until the type-C phy work has been done to make it
> usable at all?
Or alternatively just disable the cdn-dp Rockchip driver in the kernel config,
which results in it also not getting probed.
> AIUI the "official" Manjaro kernel is carrying a bunch of
> hacks to make type-C work via extcon, but they know that isn't an
> upstreamable solution.
>
> Robin.
>
> > Then, the device halts all of the DRM related device jobs. For example,
> > the operations: vop_component_ops, vop_component_ops and
> > rockchip_dp_component_ops cannot be bound to corresponding devices. So,
> > Xorg cannot find the correct DRM device.
> >
> > This patch skips the probing failed devices to fix this issue.
> >
> > Link:
> > http://lists.infradead.org/pipermail/linux-rockchip/2020-September/022352.html
> > Signed-off-by: Jian-Hong Pan
> > ---
> > drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 6 ++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> > b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> > index 0f3eb392fe39..de13588602b4 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> > @@ -331,6 +331,12 @@ static struct component_match
> > *rockchip_drm_match_add(struct device *dev)
> >
> > if (!d)
> > break;
> > + if (!d->driver) {
> > + DRM_DEV_ERROR(d,
> > + "%s did not probe successfully",
> > + drv->driver.name);
> > + continue;
> > + }
> >
> > device_link_add(dev, d, DL_FLAG_STATELESS);
> > component_match_add(dev, , compare_dev, d);
> >
>
Re: [PATCH 2/2] arm64: dts: rockchip: rk3399-khadas-edge add missed ir-receiver and ir_rx pinctl nodes
Hi Artem,
Am Mittwoch, 23. September 2020, 12:12:25 CEST schrieb Artem Lapkin:
> From: Artem Lapkin
>
> add missed ir receivier to Khadas Edge board
> Khadas Edge uses gpio-ir-receiver on RK_PB6 gpio
Missing Signed-off-by
> ---
> .../boot/dts/rockchip/rk3399-khadas-edge.dtsi| 16
> 1 file changed, 16 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-khadas-edge.dtsi
> b/arch/arm64/boot/dts/rockchip/rk3399-khadas-edge.dtsi
> index 42ebbd6fa46..389ae43d869 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-khadas-edge.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-khadas-edge.dtsi
> @@ -109,6 +109,14 @@ vsys_5v0: vsys-5v0 {
> vin-supply = <>;
> };
>
> + ir-receiver {
please sort alphabetically.
Also more importantly, is this really part of all Khadas Edge board variants?
[just making sure ;-) ]
> + compatible = "gpio-ir-receiver";
> + gpios = < RK_PB6 GPIO_ACTIVE_LOW>;
> + pinctrl-names = "default";
> + pinctrl-0 = <_rx>;
> + linux,rc-map-name = "rc-khadas";
> + };
> +
> adc-keys {
> compatible = "adc-keys";
> io-channels = < 1>;
> @@ -682,6 +690,14 @@ {
> status = "okay";
> };
>
> + {
there is already a "" node in the "rk3399-khadas-edge.dtsi"
please don't add another one, and make sure things are somewhat
sorted alphabetically ;-) .
Thanks
Heiko
> +ir {
> + ir_rx: ir-rx {
> + rockchip,pins = <1 RK_PB6 RK_FUNC_GPIO _pull_none>;
> + };
> +};
> +};
> +
> {
> bus-width = <8>;
> mmc-hs400-1_8v;
>
Re: [PATCH 1/2] arm64: dts: rockchip: rk3399-khadas-edge: add missed spiflash node
Hi Artem,
please make the subject something like
"arm64: dts: rockchip: add spiflash node to rk3399-khadas-edge"
Am Mittwoch, 23. September 2020, 12:12:24 CEST schrieb Artem Lapkin:
> From: Artem Lapkin
>
> The Khadas Edge Boards uses winbond - w25q128 spi flash with 104Mhz
Missing "Signed-off-by: ..."
> ---
> .../boot/dts/rockchip/rk3399-khadas-edge.dtsi | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-khadas-edge.dtsi
> b/arch/arm64/boot/dts/rockchip/rk3399-khadas-edge.dtsi
> index e36837c04dc..42ebbd6fa46 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-khadas-edge.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-khadas-edge.dtsi
> @@ -805,3 +805,17 @@ {
> _mmu {
> status = "okay";
> };
> +
> +
no double empty lines please
> + {
please group the alphabetically in the file
> +max-freq = <10400>;
not part of any binding, spi-max-frequency below should be enough.
> +status = "okay";
> +
> +spiflash: flash@0 {
> +u-boot,dm-pre-reloc;
u-boot specific, please drop
> +compatible = "winbond,w25q128fw", "jedec,spi-nor";
> +reg = <0>;
> +spi-max-frequency = <10400>;
> +};
> +};
> +
>
Heiko
Re: [PATCH] drm/rockchip: skip probed failed device
Hi,
Am Mittwoch, 23. September 2020, 08:59:00 CEST schrieb Jian-Hong Pan:
> The cdn-dp sub driver probes the device failed on PINEBOOK Pro.
>
> kernel: cdn-dp fec0.dp: [drm:cdn_dp_probe [rockchipdrm]] *ERROR* missing
> extcon or phy
> kernel: cdn-dp: probe of fec0.dp failed with error -22
>
> Then, the device halts all of the DRM related device jobs. For example,
> the operations: vop_component_ops, vop_component_ops and
> rockchip_dp_component_ops cannot be bound to corresponding devices. So,
> Xorg cannot find the correct DRM device.
>
> This patch skips the probing failed devices to fix this issue.
>
> Link:
> http://lists.infradead.org/pipermail/linux-rockchip/2020-September/022352.html
> Signed-off-by: Jian-Hong Pan
> ---
> drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> index 0f3eb392fe39..de13588602b4 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> @@ -331,6 +331,12 @@ static struct component_match
> *rockchip_drm_match_add(struct device *dev)
>
> if (!d)
> break;
> + if (!d->driver) {
> + DRM_DEV_ERROR(d,
> + "%s did not probe successfully",
> + drv->driver.name);
> + continue;
> + }
How does this relate to drivers doing EPROBE_DEFER?
Very often you have sub-drivers defering probe as they still need another
resource, so excluding them in that case would not work?
Heiko
>
> device_link_add(dev, d, DL_FLAG_STATELESS);
> component_match_add(dev, , compare_dev, d);
>
Re: [PATCH 7/9] i2c: rk3x: Simplify with dev_err_probe()
Am Mittwoch, 2. September 2020, 17:06:41 CEST schrieb Krzysztof Kozlowski: > Common pattern of handling deferred probe can be simplified with > dev_err_probe(). Less code and the error value gets printed. > > Signed-off-by: Krzysztof Kozlowski Reviewed-by: Heiko Stuebner
Re: [PATCH 6/6] clk: rockchip: rk3308: drop unused mux_timer_src_p
Am Mittwoch, 16. September 2020, 18:17:40 CEST schrieb Krzysztof Kozlowski: > The parent names 'mux_timer_src_p' is not used: > > In file included from drivers/clk/rockchip/clk-rk3308.c:13:0: > drivers/clk/rockchip/clk-rk3308.c:136:7: warning: ‘mux_timer_src_p’ defined > but not used [-Wunused-const-variable=] > > Signed-off-by: Krzysztof Kozlowski applied for 5.10 Thanks Heiko
Re: [PATCH] phy: rockchip-dphy-rx0: Include linux/delay.h
Am Dienstag, 22. September 2020, 00:56:18 CEST schrieb Tomasz Figa:
> Fix an implicit declaration of usleep_range():
>
> drivers/phy/rockchip/phy-rockchip-dphy-rx0.c: In function 'rk_dphy_enable':
> drivers/phy/rockchip/phy-rockchip-dphy-rx0.c:203:2: error: implicit
> declaration of function 'usleep_range' [-Werror=implicit-function-declaration]
>
> Fixes: 32abcc4491c62 ("media: staging: phy-rockchip-dphy-rx0: add Rockchip
> MIPI Synopsys DPHY RX0 driver")
> Signed-off-by: Tomasz Figa
Reviewed-by: Heiko Stuebner
Re: [PATCH] pinctrl: rockchip: populate platform device for rockchip gpio
Am Dienstag, 8. September 2020, 04:19:13 CEST schrieb Jianqun Xu:
> Register both gpio driver and device as part of driver model, so that
> the '-gpio'/'-gpios' dependency in dts can be correctly handled by
> of_devlink/of_fwlink.
>
> Signed-off-by: Jianqun Xu
Reviewed-by: Heiko Stuebner
> ---
> drivers/pinctrl/pinctrl-rockchip.c | 305 +
> 1 file changed, 175 insertions(+), 130 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-rockchip.c
> b/drivers/pinctrl/pinctrl-rockchip.c
> index c98bd352f831..2e4fc711d0d1 100644
> --- a/drivers/pinctrl/pinctrl-rockchip.c
> +++ b/drivers/pinctrl/pinctrl-rockchip.c
> @@ -3370,139 +3370,121 @@ static void rockchip_irq_disable(struct irq_data *d)
> }
>
> static int rockchip_interrupts_register(struct platform_device *pdev,
> - struct rockchip_pinctrl *info)
> + struct rockchip_pin_bank *bank)
> {
> - struct rockchip_pin_ctrl *ctrl = info->ctrl;
> - struct rockchip_pin_bank *bank = ctrl->pin_banks;
> unsigned int clr = IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_NOAUTOEN;
> struct irq_chip_generic *gc;
> int ret;
> - int i;
>
> - for (i = 0; i < ctrl->nr_banks; ++i, ++bank) {
> - if (!bank->valid) {
> - dev_warn(>dev, "bank %s is not valid\n",
> - bank->name);
> - continue;
> - }
> + if (!bank->valid) {
> + dev_warn(>dev, "bank %s is not valid\n",
> + bank->name);
> + return -EINVAL;
> + }
>
> - ret = clk_enable(bank->clk);
> - if (ret) {
> - dev_err(>dev, "failed to enable clock for bank
> %s\n",
> - bank->name);
> - continue;
> - }
> + ret = clk_enable(bank->clk);
> + if (ret) {
> + dev_err(>dev, "failed to enable clock for bank %s\n",
> + bank->name);
> + return ret;
> + }
>
> - bank->domain = irq_domain_add_linear(bank->of_node, 32,
> - _generic_chip_ops, NULL);
> - if (!bank->domain) {
> - dev_warn(>dev, "could not initialize irq domain
> for bank %s\n",
> - bank->name);
> - clk_disable(bank->clk);
> - continue;
> - }
> + bank->domain = irq_domain_add_linear(bank->of_node, 32,
> + _generic_chip_ops, NULL);
> + if (!bank->domain) {
> + dev_warn(>dev, "could not initialize irq domain for bank
> %s\n",
> + bank->name);
> + clk_disable(bank->clk);
> + return -EINVAL;
> + }
>
> - ret = irq_alloc_domain_generic_chips(bank->domain, 32, 1,
> - "rockchip_gpio_irq", handle_level_irq,
> - clr, 0, 0);
> - if (ret) {
> - dev_err(>dev, "could not alloc generic chips for
> bank %s\n",
> - bank->name);
> - irq_domain_remove(bank->domain);
> - clk_disable(bank->clk);
> - continue;
> - }
> + ret = irq_alloc_domain_generic_chips(bank->domain, 32, 1,
> + "rockchip_gpio_irq", handle_level_irq,
> + clr, 0, 0);
> + if (ret) {
> + dev_err(>dev, "could not alloc generic chips for bank
> %s\n",
> + bank->name);
> + irq_domain_remove(bank->domain);
> + clk_disable(bank->clk);
> + return ret;
> + }
>
> - gc = irq_get_domain_generic_chip(bank->domain, 0);
> - gc->reg_base = bank->reg_base;
> - gc->private = bank;
> - gc->chip_types[0].regs.mask = GPIO_INTMASK;
> - gc->chip_types[0].regs.ack = GPIO_PORTS_EOI;
> - gc->chip_types[0].chip.irq_ack = irq_gc_ack_set_bit;
> - gc->chip_types[0].chip.irq_mask = irq_gc_mask_set_bit;
> - gc->chip_types[0].chip.irq_unmask = irq_gc_mask_clr_bit;
> - gc->chip_types[0].chip.irq_enable = rockchip_irq_enable;
> - gc->chip_types[0].chip.irq_disable = rockchip_irq_disable;
> - gc->chip_types[0].chip.irq_set_wake = irq_gc_set_wake;
> - gc->chip_types[0].chip.irq_suspend = rockchip_irq_suspend;
> - gc->chip_types[0].chip.irq_resume = rockchip_irq_resume;
> - gc->chip_types[0].chip.irq_set_type = rockchip_irq_set_type;
> - gc->wake_enabled = IRQ_MSK(bank->nr_pins);
> + gc = irq_get_domain_generic_chip(bank->domain, 0);
> + gc->reg_base = bank->reg_base;
> + gc->private = bank;
> +
Re: [PATCH 2/2] pinctrl: rockchip: make driver be tristate module
Hi Jay,
Am Montag, 21. September 2020, 00:14:11 CEST schrieb Heiko Stübner:
> Am Montag, 14. September 2020, 02:38:47 CEST schrieb Jianqun Xu:
> > Make pinctrl-rockchip driver to be tristate module, support to build as
> > a module, this is useful for GKI.
> >
> > Signed-off-by: Jianqun Xu
>
> Reviewed-by: Heiko Stuebner
It seems I've reviewed all patches of this series now, but I think
you might want to resend the series a final time as v3 in a cleaned up
state (drop patch1 and just post patches 2-5 in a full series) so that
we don't confuse Linus too much with the reposted patches we currently
have.
Thanks
Heiko
>
> > ---
> > drivers/pinctrl/Kconfig| 2 +-
> > drivers/pinctrl/pinctrl-rockchip.c | 13 +
> > 2 files changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> > index 4284f39a5c61..743eb2bb8709 100644
> > --- a/drivers/pinctrl/Kconfig
> > +++ b/drivers/pinctrl/Kconfig
> > @@ -207,7 +207,7 @@ config PINCTRL_OXNAS
> > select MFD_SYSCON
> >
> > config PINCTRL_ROCKCHIP
> > - bool
> > + tristate "Rockchip gpio and pinctrl driver"
> > depends on OF
> > select PINMUX
> > select GENERIC_PINCONF
> > diff --git a/drivers/pinctrl/pinctrl-rockchip.c
> > b/drivers/pinctrl/pinctrl-rockchip.c
> > index 0401c1da79dd..927d132d6716 100644
> > --- a/drivers/pinctrl/pinctrl-rockchip.c
> > +++ b/drivers/pinctrl/pinctrl-rockchip.c
> > @@ -16,10 +16,12 @@
> > */
> >
> > #include
> > +#include
> > #include
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -4258,3 +4260,14 @@ static int __init rockchip_pinctrl_drv_register(void)
> > return platform_driver_register(_pinctrl_driver);
> > }
> > postcore_initcall(rockchip_pinctrl_drv_register);
> > +
> > +static void __exit rockchip_pinctrl_drv_unregister(void)
> > +{
> > + platform_driver_unregister(_pinctrl_driver);
> > +}
> > +module_exit(rockchip_pinctrl_drv_unregister);
> > +
> > +MODULE_DESCRIPTION("ROCKCHIP Pin Controller Driver");
> > +MODULE_LICENSE("GPL");
> > +MODULE_ALIAS("platform:pinctrl-rockchip");
> > +MODULE_DEVICE_TABLE(of, rockchip_pinctrl_dt_match);
> >
>
>
Re: [PATCH 2/2] pinctrl: rockchip: make driver be tristate module
Am Montag, 14. September 2020, 02:38:47 CEST schrieb Jianqun Xu:
> Make pinctrl-rockchip driver to be tristate module, support to build as
> a module, this is useful for GKI.
>
> Signed-off-by: Jianqun Xu
Reviewed-by: Heiko Stuebner
> ---
> drivers/pinctrl/Kconfig| 2 +-
> drivers/pinctrl/pinctrl-rockchip.c | 13 +
> 2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index 4284f39a5c61..743eb2bb8709 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -207,7 +207,7 @@ config PINCTRL_OXNAS
> select MFD_SYSCON
>
> config PINCTRL_ROCKCHIP
> - bool
> + tristate "Rockchip gpio and pinctrl driver"
> depends on OF
> select PINMUX
> select GENERIC_PINCONF
> diff --git a/drivers/pinctrl/pinctrl-rockchip.c
> b/drivers/pinctrl/pinctrl-rockchip.c
> index 0401c1da79dd..927d132d6716 100644
> --- a/drivers/pinctrl/pinctrl-rockchip.c
> +++ b/drivers/pinctrl/pinctrl-rockchip.c
> @@ -16,10 +16,12 @@
> */
>
> #include
> +#include
> #include
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -4258,3 +4260,14 @@ static int __init rockchip_pinctrl_drv_register(void)
> return platform_driver_register(_pinctrl_driver);
> }
> postcore_initcall(rockchip_pinctrl_drv_register);
> +
> +static void __exit rockchip_pinctrl_drv_unregister(void)
> +{
> + platform_driver_unregister(_pinctrl_driver);
> +}
> +module_exit(rockchip_pinctrl_drv_unregister);
> +
> +MODULE_DESCRIPTION("ROCKCHIP Pin Controller Driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:pinctrl-rockchip");
> +MODULE_DEVICE_TABLE(of, rockchip_pinctrl_dt_match);
>
Re: [PATCH 0/2] pinctrl: rockchip: codingstyle for pinctrl-rockchip
Hi Linus, Am Samstag, 12. September 2020, 13:27:44 CEST schrieb Linus Walleij: > Jianqun, Heiko, > > On Fri, Jan 17, 2020 at 9:14 AM Jianqun Xu wrote: > > > Do codingstyle for pinctrl-rockchip by spliting driver by SoC types. > > > > Convenienty for reviewing, the first patch only moving > > pinctrl-rockchip.c from driver/pinctrl to driver/pinctrl/rockchip/ . > > > > Jianqun Xu (2): > > pinctrl: rockchip: new rockchip dir for pinctrl-rockchip > > pinctrl: rockchip: split rockchip pinctrl driver by SoC type > > Why were these patches never applied? Is it my fault? It's not your fault :-) > I don't even have patch 2/2 in my mailbox, possibly it was > too big! > > Heiko if you're OK with this change can Jianqun just send a > rebased version? We agreed to split it into smaller chunks which I think is the 13-patch series you mentioned elsewhere today. But I guess that fell through the cracks in my review :-( . So I guess we should do the current GKI thingy first to get that module build and after that maybe Jianqun can find the time to rebase the per-soc split on top of that. Heiko
Re: [PATCH 2/5] pinctrl: rockchip: make driver be tristate module
Hi,
Am Montag, 7. September 2020, 04:59:24 CEST schrieb Jianqun Xu:
> Make pinctrl-rockchip driver to be tristate module, support to build as
> a module, this is useful for GKI.
>
> Signed-off-by: Jianqun Xu
> ---
> drivers/pinctrl/Kconfig| 2 +-
> drivers/pinctrl/pinctrl-rockchip.c | 18 ++
> 2 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index 4284f39a5c61..743eb2bb8709 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -207,7 +207,7 @@ config PINCTRL_OXNAS
> select MFD_SYSCON
>
> config PINCTRL_ROCKCHIP
> - bool
> + tristate "Rockchip gpio and pinctrl driver"
> depends on OF
> select PINMUX
> select GENERIC_PINCONF
> diff --git a/drivers/pinctrl/pinctrl-rockchip.c
> b/drivers/pinctrl/pinctrl-rockchip.c
> index 0401c1da79dd..cc7512acfc5f 100644
> --- a/drivers/pinctrl/pinctrl-rockchip.c
> +++ b/drivers/pinctrl/pinctrl-rockchip.c
> @@ -16,10 +16,12 @@
> */
>
> #include
> +#include
> #include
> #include
> #include
> #include
> +#include
of_device.h below of_address.h please
> #include
> #include
> #include
> @@ -4257,4 +4259,20 @@ static int __init rockchip_pinctrl_drv_register(void)
> {
> return platform_driver_register(_pinctrl_driver);
> }
> +
> +static void __exit rockchip_pinctrl_drv_unregister(void)
> +{
> + platform_driver_unregister(_pinctrl_driver);
> +}
> +
> +#ifdef CONFIG_PINCTRL_ROCKCHIP_MODULE
> +module_init(rockchip_pinctrl_drv_register);
> +#else
> postcore_initcall(rockchip_pinctrl_drv_register);
> +#endif
You definitly don't need this hack. For modules postcore_initcall
already points to module_init ... see
https://elixir.bootlin.com/linux/latest/source/include/linux/module.h#L114
Heiko
Re: [PATCH v3 6/6] clk: rockchip: rk3399: Support module build
Am Freitag, 4. September 2020, 09:45:05 CEST schrieb Elaine Zhang:
> support CLK_OF_DECLARE and builtin_platform_driver_probe
> double clk init method.
> add module author, description and license to support building
> Soc Rk3399 clock driver as module.
>
> Signed-off-by: Elaine Zhang
> Reviewed-by: Kever Yang
> ---
> drivers/clk/rockchip/clk-rk3399.c | 55 +++
> 1 file changed, 55 insertions(+)
>
> diff --git a/drivers/clk/rockchip/clk-rk3399.c
> b/drivers/clk/rockchip/clk-rk3399.c
> index ce1d2446f142..40ff17aee5b6 100644
> --- a/drivers/clk/rockchip/clk-rk3399.c
> +++ b/drivers/clk/rockchip/clk-rk3399.c
> @@ -5,9 +5,11 @@
> */
>
> #include
> +#include
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -1600,3 +1602,56 @@ static void __init rk3399_pmu_clk_init(struct
> device_node *np)
> rockchip_clk_of_add_provider(np, ctx);
> }
> CLK_OF_DECLARE(rk3399_cru_pmu, "rockchip,rk3399-pmucru",
> rk3399_pmu_clk_init);
> +
> +struct clk_rk3399_inits {
> + void (*inits)(struct device_node *np);
> +};
> +
> +static const struct clk_rk3399_inits clk_rk3399_pmucru_init = {
> + .inits = rk3399_pmu_clk_init,
> +};
> +
> +static const struct clk_rk3399_inits clk_rk3399_cru_init = {
> + .inits = rk3399_clk_init,
> +};
> +
> +static const struct of_device_id clk_rk3399_match_table[] = {
> + {
> + .compatible = "rockchip,rk3399-cru",
> + .data = _rk3399_cru_init,
> + }, {
> + .compatible = "rockchip,rk3399-pmucru",
> + .data = _rk3399_pmucru_init,
> + },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, clk_rk3399_match_table);
> +
> +static int __init clk_rk3399_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + const struct of_device_id *match;
> + const struct clk_rk3399_inits *init_data;
> +
> + match = of_match_device(clk_rk3399_match_table, >dev);
> + if (!match || !match->data)
> + return -EINVAL;
> +
> + init_data = match->data;
> + if (init_data->inits)
> + init_data->inits(np);
> +
> + return 0;
> +}
> +
> +static struct platform_driver clk_rk3399_driver = {
> + .driver = {
> + .name = "clk-rk3399",
> + .of_match_table = clk_rk3399_match_table,
I guess we probably want
.suppress_bind_attrs = true,
here, because there is no unloading.
Also what happens when you try to rmmod the module?
Heiko
Re: [PATCH v3 1/6] clk: rockchip: Use clk_hw_register_composite instead of clk_register_composite calls
Am Freitag, 4. September 2020, 09:44:00 CEST schrieb Elaine Zhang: > clk_hw_register_composite it's already exported. > Preparation for compilation of rK common clock drivers into modules. > > Signed-off-by: Elaine Zhang > Reported-by: kernel test robot > Reviewed-by: Kever Yang Reviewed-by: Heiko Stuebner
Re: [PATCH v3 4/6] clk: rockchip: Export some clock common APIs for module drivers
Am Freitag, 4. September 2020, 09:44:03 CEST schrieb Elaine Zhang:
> This is used by the Rockchip clk driver, export it to allow that
> driver to be compiled as a module.
>
> Signed-off-by: Elaine Zhang
> Reviewed-by: Kever Yang
> ---
> drivers/clk/rockchip/clk.c | 52 ++
> 1 file changed, 30 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/clk/rockchip/clk.c b/drivers/clk/rockchip/clk.c
> index 46409972983e..fd3aff2a599d 100644
> --- a/drivers/clk/rockchip/clk.c
> +++ b/drivers/clk/rockchip/clk.c
> @@ -360,8 +360,9 @@ static struct clk
> *rockchip_clk_register_factor_branch(const char *name,
> return hw->clk;
> }
>
> -struct rockchip_clk_provider * __init rockchip_clk_init(struct device_node
> *np,
> - void __iomem *base, unsigned long nr_clks)
> +struct rockchip_clk_provider *rockchip_clk_init(struct device_node *np,
> + void __iomem *base,
> + unsigned long nr_clks)
> {
> struct rockchip_clk_provider *ctx;
> struct clk **clk_table;
> @@ -393,14 +394,16 @@ struct rockchip_clk_provider * __init
> rockchip_clk_init(struct device_node *np,
> kfree(ctx);
> return ERR_PTR(-ENOMEM);
> }
> +EXPORT_SYMBOL(rockchip_clk_init);
again, same comment about EXPORT_SYMBOL_GPL
Re: [PATCH v3 5/6] clk: rockchip: fix the clk config to support module build
Hi Elaine, Am Freitag, 4. September 2020, 09:44:48 CEST schrieb Elaine Zhang: > use CONFIG_COMMON_CLK_ROCKCHIP for Rk common clk drivers. > use CONFIG_CLK_RKXX for Rk soc clk driver. > Mark configuration to "tristate", > to support building Rk SoCs clock driver as module. > > Signed-off-by: Elaine Zhang > Reviewed-by: Kever Yang > --- > drivers/clk/Kconfig | 1 + > drivers/clk/rockchip/Kconfig | 78 +++ > drivers/clk/rockchip/Makefile | 42 ++- > 3 files changed, 101 insertions(+), 20 deletions(-) > create mode 100644 drivers/clk/rockchip/Kconfig > > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig > index 4026fac9fac3..b41aaed9bd51 100644 > --- a/drivers/clk/Kconfig > +++ b/drivers/clk/Kconfig > @@ -373,6 +373,7 @@ source "drivers/clk/meson/Kconfig" > source "drivers/clk/mvebu/Kconfig" > source "drivers/clk/qcom/Kconfig" > source "drivers/clk/renesas/Kconfig" > +source "drivers/clk/rockchip/Kconfig" > source "drivers/clk/samsung/Kconfig" > source "drivers/clk/sifive/Kconfig" > source "drivers/clk/sprd/Kconfig" > diff --git a/drivers/clk/rockchip/Kconfig b/drivers/clk/rockchip/Kconfig > new file mode 100644 > index ..53a44396bc35 > --- /dev/null > +++ b/drivers/clk/rockchip/Kconfig > @@ -0,0 +1,78 @@ > +# SPDX-License-Identifier: GPL-2.0 > +# common clock support for ROCKCHIP SoC family. > + > +config COMMON_CLK_ROCKCHIP > + tristate "Rockchip clock controller common support" > + depends on ARCH_ROCKCHIP > + default ARCH_ROCKCHIP > + help > + Say y here to enable common clock controller. Maybe "Say y here to enable common clock controller for Rockchip platforms." > + > +if COMMON_CLK_ROCKCHIP > +config CLK_PX30 > + tristate "Rockchip Px30 clock controller support" 2 Problems: - order: you add the symbols allowing module builds before adding module infrastructure, so in a bisection build with patch6 not applied you probably end up with build errors? - in patch6 you only add module infrastructure for rk3399, but here you declare all as tristate ... so what happens if someone selects to build the PX30 as module now? Thanks Heiko > + default y > + help > + Build the driver for Px30 Clock Driver. > + > +config CLK_RV110X > + tristate "Rockchip Rv110x clock controller support" > + default y > + help > + Build the driver for Rv110x Clock Driver. > + > +config CLK_RK3036 > + tristate "Rockchip Rk3036 clock controller support" > + default y > + help > + Build the driver for Rk3036 Clock Driver. > + > +config CLK_RK312X > + tristate "Rockchip Rk312x clock controller support" > + default y > + help > + Build the driver for Rk312x Clock Driver. > + > +config CLK_RK3188 > + tristate "Rockchip Rk3188 clock controller support" > + default y > + help > + Build the driver for Rk3188 Clock Driver. > + > +config CLK_RK322X > + tristate "Rockchip Rk322x clock controller support" > + default y > + help > + Build the driver for Rk322x Clock Driver. > + > +config CLK_RK3288 > + tristate "Rockchip Rk3288 clock controller support" > + depends on ARM > + default y > + help > + Build the driver for Rk3288 Clock Driver. > + > +config CLK_RK3308 > + tristate "Rockchip Rk3308 clock controller support" > + default y > + help > + Build the driver for Rk3308 Clock Driver. > + > +config CLK_RK3328 > + tristate "Rockchip Rk3328 clock controller support" > + default y > + help > + Build the driver for Rk3328 Clock Driver. > + > +config CLK_RK3368 > + tristate "Rockchip Rk3368 clock controller support" > + default y > + help > + Build the driver for Rk3368 Clock Driver. > + > +config CLK_RK3399 > + tristate "Rockchip Rk3399 clock controller support" > + default y > + help > + Build the driver for Rk3399 Clock Driver. > +endif > diff --git a/drivers/clk/rockchip/Makefile b/drivers/clk/rockchip/Makefile > index 7c5b5813a87c..a99e4d9bbae1 100644 > --- a/drivers/clk/rockchip/Makefile > +++ b/drivers/clk/rockchip/Makefile > @@ -3,24 +3,26 @@ > # Rockchip Clock specific Makefile > # > > -obj-y+= clk.o > -obj-y+= clk-pll.o > -obj-y+= clk-cpu.o > -obj-y+= clk-half-divider.o > -obj-y+= clk-inverter.o > -obj-y+= clk-mmc-phase.o > -obj-y+= clk-muxgrf.o > -obj-y+= clk-ddr.o > -obj-$(CONFIG_RESET_CONTROLLER) += softrst.o > +obj-$(CONFIG_COMMON_CLK_ROCKCHIP) += clk-rockchip.o > > -obj-y+= clk-px30.o > -obj-y+= clk-rv1108.o > -obj-y+= clk-rk3036.o > -obj-y+= clk-rk3128.o > -obj-y+= clk-rk3188.o > -obj-y+= clk-rk3228.o > -obj-y+= clk-rk3288.o > -obj-y+= clk-rk3308.o > -obj-y+= clk-rk3328.o > -obj-y+= clk-rk3368.o > -obj-y+= clk-rk3399.o > +clk-rockchip-y += clk.o >
Re: [PATCH v3 3/6] clk: rockchip: Export rockchip_register_softrst()
Am Freitag, 4. September 2020, 09:44:02 CEST schrieb Elaine Zhang:
> This is used by the Rockchip clk driver, export it to allow that
> driver to be compiled as a module..
>
> Signed-off-by: Elaine Zhang
> Reviewed-by: Kever Yang
> ---
> drivers/clk/rockchip/softrst.c | 7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/clk/rockchip/softrst.c b/drivers/clk/rockchip/softrst.c
> index 5f1ff5e47c4f..caba9055090b 100644
> --- a/drivers/clk/rockchip/softrst.c
> +++ b/drivers/clk/rockchip/softrst.c
> @@ -77,9 +77,9 @@ static const struct reset_control_ops rockchip_softrst_ops
> = {
> .deassert = rockchip_softrst_deassert,
> };
>
> -void __init rockchip_register_softrst(struct device_node *np,
> - unsigned int num_regs,
> - void __iomem *base, u8 flags)
> +void rockchip_register_softrst(struct device_node *np,
> +unsigned int num_regs,
> +void __iomem *base, u8 flags)
> {
> struct rockchip_softrst *softrst;
> int ret;
> @@ -107,3 +107,4 @@ void __init rockchip_register_softrst(struct device_node
> *np,
> kfree(softrst);
> }
> };
> +EXPORT_SYMBOL(rockchip_register_softrst);
Same comment about EXPORT_SYMBOL_GPL perhaps?
Re: [PATCH v3 2/6] clk: rockchip: Export rockchip_clk_register_ddrclk()
Am Freitag, 4. September 2020, 09:44:01 CEST schrieb Elaine Zhang: > This is used by the Rockchip clk driver, export it to allow that > driver to be compiled as a module.. > > Signed-off-by: Elaine Zhang > Reviewed-by: Kever Yang > --- > drivers/clk/rockchip/clk-ddr.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/clk/rockchip/clk-ddr.c b/drivers/clk/rockchip/clk-ddr.c > index 9273bce4d7b6..282b6f22eb22 100644 > --- a/drivers/clk/rockchip/clk-ddr.c > +++ b/drivers/clk/rockchip/clk-ddr.c > @@ -136,3 +136,4 @@ struct clk *rockchip_clk_register_ddrclk(const char > *name, int flags, > > return clk; > } > +EXPORT_SYMBOL(rockchip_clk_register_ddrclk); EXPORT_SYMBOL_GPL perhaps? The rest of clock-framework exports are already the _GPL variant anyway, so ours should probably be too.
Re: [PATCH 6/6] pinctrl: rockchip: populate platform device for rockchip gpio
Hi Jianqun,
Am Montag, 31. August 2020, 10:50:21 CEST schrieb Jianqun Xu:
> Register both gpio driver and device as part of driver model, so that
> the '-gpio'/'-gpios' dependency in dts can be correctly handled by
> of_devlink/of_fwlink.
>
> Signed-off-by: Jianqun Xu
[...]
> -static int rockchip_gpiolib_unregister(struct platform_device *pdev,
> - struct rockchip_pinctrl *info)
> -{
> - struct rockchip_pin_ctrl *ctrl = info->ctrl;
> - struct rockchip_pin_bank *bank = ctrl->pin_banks;
> - int i;
> + pctldev = info->pctl_dev;
> + if (!pctldev)
> + return -ENODEV;
>
> - for (i = 0; i < ctrl->nr_banks; ++i, ++bank) {
> - if (!bank->valid)
> - continue;
> - gpiochip_remove(>gpio_chip);
> + ret = gpiochip_add_pin_range(gc, dev_name(pctldev->dev), 0,
> gc->base, gc->ngpio);
> + if (ret) {
> + dev_err(>dev, "Failed to add pin range\n");
> + gpiochip_remove(>gpio_chip);
> + return ret;
> + }
> }
>
> return 0;
> @@ -3752,6 +3734,46 @@ static int __maybe_unused
> rockchip_pinctrl_resume(struct device *dev)
> static SIMPLE_DEV_PM_OPS(rockchip_pinctrl_dev_pm_ops,
> rockchip_pinctrl_suspend,
>rockchip_pinctrl_resume);
>
> +static int rockchip_gpio_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct device *parent = pdev->dev.parent;
> + struct rockchip_pinctrl *info = dev_get_drvdata(parent);
> + struct rockchip_pin_ctrl *ctrl = info ? (info->ctrl) : NULL;
> + struct rockchip_pin_bank *bank;
> + int ret, i;
> +
> + if (!info || !ctrl)
> + return -EINVAL;
> +
> + if (!of_find_property(np, "gpio-controller", NULL))
> + return -ENODEV;
> +
> + bank = ctrl->pin_banks;
> + for (i = 0; i < ctrl->nr_banks; ++i, ++bank) {
> + if (!strcmp(bank->name, np->name)) {
> + bank->of_node = np;
> +
> + if (!rockchip_get_bank_data(bank, info))
> + bank->valid = true;
> +
> + break;
> + }
> + }
> +
> + bank->of_node = pdev->dev.of_node;
> +
> + ret = rockchip_gpiolib_register(pdev, bank);
> + if (ret)
> + return ret;
> +
> + ret = rockchip_interrupts_register(pdev, bank);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> static int rockchip_pinctrl_probe(struct platform_device *pdev)
> {
> struct rockchip_pinctrl *info;
> @@ -3823,18 +3845,20 @@ static int rockchip_pinctrl_probe(struct
> platform_device *pdev)
> return PTR_ERR(info->regmap_pmu);
> }
>
> - ret = rockchip_gpiolib_register(pdev, info);
> - if (ret)
> - return ret;
> -
> ret = rockchip_pinctrl_register(pdev, info);
> if (ret) {
> - rockchip_gpiolib_unregister(pdev, info);
> + dev_err(>dev, "failed to register pinctrl device\n");
> return ret;
> }
>
> platform_set_drvdata(pdev, info);
>
> + ret = of_platform_populate(np, rockchip_bank_match, NULL, dev);
> + if (ret) {
> + dev_err(>dev, "failed to register gpio device\n");
> + return ret;
> + }
> +
> return 0;
> }
>
> @@ -4254,6 +4278,14 @@ static const struct of_device_id
> rockchip_pinctrl_dt_match[] = {
> {},
> };
>
> +static struct platform_driver rockchip_gpio_driver = {
> + .probe = rockchip_gpio_probe,
> + .driver = {
> + .name = "rockchip-gpio",
> + .of_match_table = rockchip_bank_match,
> + },
> +};
> +
> static struct platform_driver rockchip_pinctrl_driver = {
> .probe = rockchip_pinctrl_probe,
> .driver = {
> @@ -4265,7 +4297,9 @@ static struct platform_driver rockchip_pinctrl_driver =
> {
>
> static int __init rockchip_pinctrl_drv_register(void)
> {
> - return platform_driver_register(_pinctrl_driver);
> + platform_driver_register(_pinctrl_driver);
> +
> + return platform_driver_register(_gpio_driver);
> }
> postcore_initcall(rockchip_pinctrl_drv_register);
This also seems to miss the whole unloading of module paths?
I'd expect a gpiochip_remove etc in some _remove function and
of course a platform_driver_unregister.
Heiko
Re: [PATCH 1/6] pinctrl: rockchip: make driver be tristate module
Am Sonntag, 6. September 2020, 00:01:55 CEST schrieb Heiko Stübner:
> Am Montag, 31. August 2020, 10:47:48 CEST schrieb Jianqun Xu:
> > Make pinctrl-rockchip driver to be tristate module, support to build as
> > a module, this is useful for GKI.
> >
> > Signed-off-by: Jianqun Xu
>
> Reviewed-by: Heiko Stuebner
I take this back.
What happens when you actually unload the module now?
I checked and all the pinctrl stuff itself is using devm-functions
so should be safe, but you're missing the
platform_driver_unregister part that should happen as well.
Heiko
> > ---
> > drivers/pinctrl/Kconfig| 2 +-
> > drivers/pinctrl/pinctrl-rockchip.c | 7 +++
> > 2 files changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> > index 8828613c4e0e..dd4874e2ac67 100644
> > --- a/drivers/pinctrl/Kconfig
> > +++ b/drivers/pinctrl/Kconfig
> > @@ -207,7 +207,7 @@ config PINCTRL_OXNAS
> > select MFD_SYSCON
> >
> > config PINCTRL_ROCKCHIP
> > - bool
> > + tristate "Rockchip gpio and pinctrl driver"
> > select PINMUX
> > select GENERIC_PINCONF
> > select GENERIC_IRQ_CHIP
> > diff --git a/drivers/pinctrl/pinctrl-rockchip.c
> > b/drivers/pinctrl/pinctrl-rockchip.c
> > index c07324d1f265..24dfc814dee1 100644
> > --- a/drivers/pinctrl/pinctrl-rockchip.c
> > +++ b/drivers/pinctrl/pinctrl-rockchip.c
> > @@ -16,10 +16,12 @@
> > */
> >
> > #include
> > +#include
> > #include
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -4256,3 +4258,8 @@ static int __init rockchip_pinctrl_drv_register(void)
> > return platform_driver_register(_pinctrl_driver);
> > }
> > postcore_initcall(rockchip_pinctrl_drv_register);
> > +
> > +MODULE_DESCRIPTION("ROCKCHIP Pin Controller Driver");
> > +MODULE_LICENSE("GPL");
> > +MODULE_ALIAS("platform:pinctrl-rockchip");
> > +MODULE_DEVICE_TABLE(of, rockchip_pinctrl_dt_match);
> >
>
>
Re: [PATCH 5/6] pinctrl: rockchip: fix crash caused by invalid gpio bank
Hi,
Am Montag, 31. August 2020, 10:50:10 CEST schrieb Jianqun Xu:
> Add valid check for gpio bank.
Please add more description on where this happened.
> Change-Id: Ia4609c3045b5df7879beab3c15d791ff54a1f49b
Please drop the change-id.
> Signed-off-by: Jianqun Xu
> ---
> drivers/pinctrl/pinctrl-rockchip.c | 10 --
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-rockchip.c
> b/drivers/pinctrl/pinctrl-rockchip.c
> index 6080573155f6..5b16b69e311f 100644
> --- a/drivers/pinctrl/pinctrl-rockchip.c
> +++ b/drivers/pinctrl/pinctrl-rockchip.c
> @@ -2526,9 +2526,9 @@ static int rockchip_pmx_set(struct pinctrl_dev
> *pctldev, unsigned selector,
> break;
> }
>
> - if (ret) {
> + if (ret && cnt) {
> /* revert the already done pin settings */
> - for (cnt--; cnt >= 0; cnt--)
> + for (cnt--; cnt >= 0 && !data[cnt].func; cnt--)
This looks unrelated and as it's not a "check for a valid gpio-bank" it
should become a separate patch with a commit message describing it nicely.
> rockchip_set_mux(bank, pins[cnt] - bank->pin_base, 0);
>
> return ret;
> @@ -2599,9 +2599,13 @@ static int rockchip_pmx_gpio_set_direction(struct
> pinctrl_dev *pctldev,
> unsigned offset, bool input)
> {
> struct rockchip_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
> + struct rockchip_pin_bank *bank = >ctrl->pin_banks[offset / 32];
> struct gpio_chip *chip;
> int pin;
>
> + if (!bank || !bank->valid)
> + return 0;
> +
> chip = range->gc;
> pin = offset - chip->base;
> dev_dbg(info->dev, "gpio_direction for pin %u as %s-%d to %s\n",
> @@ -3022,6 +3026,8 @@ static int rockchip_pinctrl_register(struct
> platform_device *pdev,
>
> for (bank = 0; bank < info->ctrl->nr_banks; ++bank) {
> pin_bank = >ctrl->pin_banks[bank];
> + if (!pin_bank->valid)
> + continue;
Please add a blank line here
> pin_bank->grange.name = pin_bank->name;
> pin_bank->grange.id = bank;
> pin_bank->grange.pin_base = pin_bank->pin_base;
>
Thanks
Heiko
Re: [PATCH 4/6] pinctrl: rockchip: do not set gpio if bank invalid
Am Montag, 31. August 2020, 10:47:51 CEST schrieb Jianqun Xu: > Add valid check for gpio bank. As this obviously fixes a problem you encountered please elaborate a bit more. Just so that people reading the log later understand when this issue surfaced. Also - maybe even more important - why is this limited to PIN_CONFIG_OUTPUT? Like when the whole bank is not valid, you should be able to return the -ENOTSUPP even before entering the "for" loop in these functions. > Change-Id: Ib03e2910a7316bd61df18236151e371c4d04077a Please remove the changeId. Thanks Heiko > Signed-off-by: Jianqun Xu > --- > drivers/pinctrl/pinctrl-rockchip.c | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/pinctrl/pinctrl-rockchip.c > b/drivers/pinctrl/pinctrl-rockchip.c > index 265d64b8c4f5..6080573155f6 100644 > --- a/drivers/pinctrl/pinctrl-rockchip.c > +++ b/drivers/pinctrl/pinctrl-rockchip.c > @@ -2687,6 +2687,9 @@ static int rockchip_pinconf_set(struct pinctrl_dev > *pctldev, unsigned int pin, > return rc; > break; > case PIN_CONFIG_OUTPUT: > + if (!bank->valid) > + return -ENOTSUPP; > + > rockchip_gpio_set(>gpio_chip, > pin - bank->pin_base, arg); > rc = _rockchip_pmx_gpio_set_direction(>gpio_chip, > @@ -2752,6 +2755,9 @@ static int rockchip_pinconf_get(struct pinctrl_dev > *pctldev, unsigned int pin, > arg = 1; > break; > case PIN_CONFIG_OUTPUT: > + if (!bank->valid) > + return -ENOTSUPP; > + > rc = rockchip_get_mux(bank, pin - bank->pin_base); > if (rc != RK_FUNC_GPIO) > return -EINVAL; >
Re: [PATCH 3/6] pinctrl: rockchip: create irq mapping in gpio_to_irq
Am Montag, 31. August 2020, 10:47:50 CEST schrieb Jianqun Xu:
> Remove totally irq mappings create in probe, the gpio irq mapping will
> be created when do
> gpio_to_irq ->
> rockchip_gpio_to_irq ->
> irq_create_mapping
>
> This patch can speed up system boot on, also abandon many unused irq
> mappings' create.
>
> Signed-off-by: Jianqun Xu
Reviewed-by: Heiko Stuebner
> ---
> drivers/pinctrl/pinctrl-rockchip.c | 28
> 1 file changed, 12 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-rockchip.c
> b/drivers/pinctrl/pinctrl-rockchip.c
> index 54abda7b7be8..265d64b8c4f5 100644
> --- a/drivers/pinctrl/pinctrl-rockchip.c
> +++ b/drivers/pinctrl/pinctrl-rockchip.c
> @@ -3196,7 +3196,7 @@ static void rockchip_irq_demux(struct irq_desc *desc)
>
> irq = __ffs(pend);
> pend &= ~BIT(irq);
> - virq = irq_linear_revmap(bank->domain, irq);
> + virq = irq_find_mapping(bank->domain, irq);
>
> if (!virq) {
> dev_err(bank->drvdata->dev, "unmapped irq %d\n", irq);
> @@ -3375,7 +3375,7 @@ static int rockchip_interrupts_register(struct
> platform_device *pdev,
> unsigned int clr = IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_NOAUTOEN;
> struct irq_chip_generic *gc;
> int ret;
> - int i, j;
> + int i;
>
> for (i = 0; i < ctrl->nr_banks; ++i, ++bank) {
> if (!bank->valid) {
> @@ -3402,7 +3402,7 @@ static int rockchip_interrupts_register(struct
> platform_device *pdev,
>
> ret = irq_alloc_domain_generic_chips(bank->domain, 32, 1,
>"rockchip_gpio_irq", handle_level_irq,
> - clr, 0, IRQ_GC_INIT_MASK_CACHE);
> + clr, 0, 0);
> if (ret) {
> dev_err(>dev, "could not alloc generic chips for
> bank %s\n",
> bank->name);
> @@ -3411,14 +3411,6 @@ static int rockchip_interrupts_register(struct
> platform_device *pdev,
> continue;
> }
>
> - /*
> - * Linux assumes that all interrupts start out disabled/masked.
> - * Our driver only uses the concept of masked and always keeps
> - * things enabled, so for us that's all masked and all enabled.
> - */
> - writel_relaxed(0x, bank->reg_base + GPIO_INTMASK);
> - writel_relaxed(0x, bank->reg_base + GPIO_INTEN);
> -
> gc = irq_get_domain_generic_chip(bank->domain, 0);
> gc->reg_base = bank->reg_base;
> gc->private = bank;
> @@ -3435,13 +3427,17 @@ static int rockchip_interrupts_register(struct
> platform_device *pdev,
> gc->chip_types[0].chip.irq_set_type = rockchip_irq_set_type;
> gc->wake_enabled = IRQ_MSK(bank->nr_pins);
>
> + /*
> + * Linux assumes that all interrupts start out disabled/masked.
> + * Our driver only uses the concept of masked and always keeps
> + * things enabled, so for us that's all masked and all enabled.
> + */
> + writel_relaxed(0x, bank->reg_base + GPIO_INTMASK);
> + writel_relaxed(0x, bank->reg_base + GPIO_INTEN);
> + gc->mask_cache = 0x;
> +
> irq_set_chained_handler_and_data(bank->irq,
>rockchip_irq_demux, bank);
> -
> - /* map the gpio irqs here, when the clock is still running */
> - for (j = 0 ; j < 32 ; j++)
> - irq_create_mapping(bank->domain, j);
> -
> clk_disable(bank->clk);
> }
>
>
Re: [PATCH 1/6] pinctrl: rockchip: make driver be tristate module
Am Montag, 31. August 2020, 10:47:48 CEST schrieb Jianqun Xu:
> Make pinctrl-rockchip driver to be tristate module, support to build as
> a module, this is useful for GKI.
>
> Signed-off-by: Jianqun Xu
Reviewed-by: Heiko Stuebner
> ---
> drivers/pinctrl/Kconfig| 2 +-
> drivers/pinctrl/pinctrl-rockchip.c | 7 +++
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index 8828613c4e0e..dd4874e2ac67 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -207,7 +207,7 @@ config PINCTRL_OXNAS
> select MFD_SYSCON
>
> config PINCTRL_ROCKCHIP
> - bool
> + tristate "Rockchip gpio and pinctrl driver"
> select PINMUX
> select GENERIC_PINCONF
> select GENERIC_IRQ_CHIP
> diff --git a/drivers/pinctrl/pinctrl-rockchip.c
> b/drivers/pinctrl/pinctrl-rockchip.c
> index c07324d1f265..24dfc814dee1 100644
> --- a/drivers/pinctrl/pinctrl-rockchip.c
> +++ b/drivers/pinctrl/pinctrl-rockchip.c
> @@ -16,10 +16,12 @@
> */
>
> #include
> +#include
> #include
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -4256,3 +4258,8 @@ static int __init rockchip_pinctrl_drv_register(void)
> return platform_driver_register(_pinctrl_driver);
> }
> postcore_initcall(rockchip_pinctrl_drv_register);
> +
> +MODULE_DESCRIPTION("ROCKCHIP Pin Controller Driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:pinctrl-rockchip");
> +MODULE_DEVICE_TABLE(of, rockchip_pinctrl_dt_match);
>
Re: [PATCH 2/6] pinctrl: rockchip: enable gpio pclk for rockchip_gpio_to_irq
Am Montag, 31. August 2020, 10:47:49 CEST schrieb Jianqun Xu: > There need to enable pclk_gpio when do irq_create_mapping, since it will > do access to gpio controller. > > Signed-off-by: Jianqun Xu Reviewed-by: Heiko Stuebner > --- > drivers/pinctrl/pinctrl-rockchip.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/pinctrl/pinctrl-rockchip.c > b/drivers/pinctrl/pinctrl-rockchip.c > index 24dfc814dee1..54abda7b7be8 100644 > --- a/drivers/pinctrl/pinctrl-rockchip.c > +++ b/drivers/pinctrl/pinctrl-rockchip.c > @@ -3155,7 +3155,9 @@ static int rockchip_gpio_to_irq(struct gpio_chip *gc, > unsigned offset) > if (!bank->domain) > return -ENXIO; > > + clk_enable(bank->clk); > virq = irq_create_mapping(bank->domain, offset); > + clk_disable(bank->clk); > > return (virq) ? : -ENXIO; > } >
Re: [PATCH 1/6] pinctrl: rockchip: make driver be tristate module
Hi,
Am Dienstag, 1. September 2020, 12:13:16 CEST schrieb kernel test robot:
> Hi Jianqun,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on rockchip/for-next]
> [also build test ERROR on pinctrl/devel v5.9-rc3 next-20200828]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url:
> https://github.com/0day-ci/linux/commits/Jianqun-Xu/rockchip-pinctrl-fixes-for-GKI/20200831-165040
> base:
> https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git
> for-next
> config: x86_64-randconfig-m031-20200901 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
> reproduce (this is a W=1 build):
> # save the attached .config to linux build tree
> make W=1 ARCH=x86_64
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot
>
> All errors (new ones prefixed by >>):
>
>drivers/pinctrl/pinctrl-rockchip.c: In function
> 'rockchip_pinctrl_parse_groups':
> >> drivers/pinctrl/pinctrl-rockchip.c:2881:9: error: implicit declaration of
> >> function 'pinconf_generic_parse_dt_config'; did you mean
> >> 'pinconf_generic_dump_config'? [-Werror=implicit-function-declaration]
> 2881 | ret = pinconf_generic_parse_dt_config(np_config, NULL,
> | ^~~
> | pinconf_generic_dump_config
>drivers/pinctrl/pinctrl-rockchip.c: In function
> 'rockchip_gpiolib_register':
> >> drivers/pinctrl/pinctrl-rockchip.c:3473:5: error: 'struct gpio_chip' has
> >> no member named 'of_node'
> 3473 | gc->of_node = bank->of_node;
> | ^~
>At top level:
>drivers/pinctrl/pinctrl-rockchip.c:2804:34: warning: 'rockchip_bank_match'
> defined but not used [-Wunused-const-variable=]
> 2804 | static const struct of_device_id rockchip_bank_match[] = {
> | ^~~
>cc1: some warnings being treated as errors
these errors are unrelated to this patch, and I addressed them in
[PATCH] pinctrl: rockchip: depend on OF [0]
Heiko
[0] http://lore.kernel.org/r/20200905214955.907950-1-he...@sntech.de
>
> #
> https://github.com/0day-ci/linux/commit/38fa905767d010bbbc1035b48494d4a83bb72410
> git remote add linux-review https://github.com/0day-ci/linux
> git fetch --no-tags linux-review
> Jianqun-Xu/rockchip-pinctrl-fixes-for-GKI/20200831-165040
> git checkout 38fa905767d010bbbc1035b48494d4a83bb72410
> vim +2881 drivers/pinctrl/pinctrl-rockchip.c
>
> d3e5116119bd02 Heiko Stübner 2013-06-10 2823
> d3e5116119bd02 Heiko Stübner 2013-06-10 2824 static int
> rockchip_pinctrl_parse_groups(struct device_node *np,
> d3e5116119bd02 Heiko Stübner 2013-06-10 2825
> struct rockchip_pin_group *grp,
> d3e5116119bd02 Heiko Stübner 2013-06-10 2826
> struct rockchip_pinctrl *info,
> d3e5116119bd02 Heiko Stübner 2013-06-10 2827
> u32 index)
> d3e5116119bd02 Heiko Stübner 2013-06-10 2828 {
> d3e5116119bd02 Heiko Stübner 2013-06-10 2829 struct
> rockchip_pin_bank *bank;
> d3e5116119bd02 Heiko Stübner 2013-06-10 2830 int size;
> d3e5116119bd02 Heiko Stübner 2013-06-10 2831 const __be32 *list;
> d3e5116119bd02 Heiko Stübner 2013-06-10 2832 int num;
> d3e5116119bd02 Heiko Stübner 2013-06-10 2833 int i, j;
> d3e5116119bd02 Heiko Stübner 2013-06-10 2834 int ret;
> d3e5116119bd02 Heiko Stübner 2013-06-10 2835
> 94f4e54cecaf3e Rob Herring 2018-08-27 2836 dev_dbg(info->dev,
> "group(%d): %pOFn\n", index, np);
> d3e5116119bd02 Heiko Stübner 2013-06-10 2837
> d3e5116119bd02 Heiko Stübner 2013-06-10 2838 /* Initialise group */
> d3e5116119bd02 Heiko Stübner 2013-06-10 2839 grp->name = np->name;
> d3e5116119bd02 Heiko Stübner 2013-06-10 2840
> d3e5116119bd02 Heiko Stübner 2013-06-10 2841 /*
> d3e5116119bd02 Heiko Stübner 2013-06-10 2842* the binding format
> is rockchip,pins = ,
> d3e5116119bd02 Heiko Stübner 2013-06-10 2843* do sanity check and
> calculate pins number
> d3e5116119bd02 Heiko Stübner 2013-06-10 2844*/
> d3e5116119bd02 Heiko Stübner 2013-06-10 2845 list =
> of_get_property(np, "rockchip,pins", );
> d3e5116119bd02 Heiko Stübner 2013-06-10 2846 /* we do not check
> return since it's safe node passed down
Re: [PATCH 0/3] extcon: Add Type-C Virtual PD
Hi Jagan, Am Freitag, 4. September 2020, 21:18:27 CEST schrieb Jagan Teki: > USB Type-C protocol supports various modes of operations > includes PD, USB3, and Altmode. If the platform design > supports a Type-C connector then configuring these modes > can be done via enumeration. > > However, there are some platforms that design these modes > of operations as separate protocol connectors like design > Display Port from on-chip USB3 controller. So accessing > Type-C Altmode Display Port via onboard Display Port > connector instead of a Type-C connector. > > These kinds of platforms require an explicit extcon driver > in order to handle Power Delivery and Port Detection. > > This series support this Type-C Virtual PD and enable the > same in ROCK Pi 4C SBC. > > Any inputs? I tend to disagree on the design via an extcon. That the Rockchip rk3399 currently carries that extcon thingy is unfortunate and only works for ChromeOS devices based on the rk3399. The kernel now has a real type-c framework so we should not extend this extcon hack any further, because that will make it even harder to roll back later. Also simply because other Rockchip boards currently can't really make use of type-c due to this, as they use the fsusb302 phys directly connected. ChromeOS actually spend some time to make the cros-ec pd part of the type-c framework if I remember correctly, so a viable battle plan would be to: (1) move the Rockchip type-c phy driver to actually be part of the type-c framework, with the extcon being a deprecated fallback for old DTs. (2) implement your gpio-altmode as part of the type-c framework (which may even already exist) In short, please don't extend the rk3399 type-c extcon hack. Thanks Heiko
Re: [PATCH] iio: adc: rockchip_saradc: Allow compile-testing with !ARM
Am Freitag, 4. September 2020, 19:04:16 CEST schrieb Alex Dewar: > There seems no reason to allow for compile-testing on ARM only, so > remove this restriction. > > Build-tested with allyesconfig on x86. > > Signed-off-by: Alex Dewar Reviewed-by: Heiko Stuebner > --- > drivers/iio/adc/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > index d94dc800b842..03929606bb01 100644 > --- a/drivers/iio/adc/Kconfig > +++ b/drivers/iio/adc/Kconfig > @@ -863,7 +863,7 @@ config RN5T618_ADC > > config ROCKCHIP_SARADC > tristate "Rockchip SARADC driver" > - depends on ARCH_ROCKCHIP || (ARM && COMPILE_TEST) > + depends on ARCH_ROCKCHIP || COMPILE_TEST > depends on RESET_CONTROLLER > select IIO_BUFFER > select IIO_TRIGGERED_BUFFER >
Re: [PATCH v2 3/6] dt-bindings: arm: rockchip: Add ROCKPi 4B binding
Am Donnerstag, 23. Juli 2020, 08:34:12 CEST schrieb Jagan Teki: > Hi Heiko, > > On Thu, Jul 23, 2020 at 4:43 AM Heiko Stuebner wrote: > > > > Hi Jagan, > > > > Am Mittwoch, 22. Juli 2020, 21:09:46 CEST schrieb Jagan Teki: > > > Add dt-bindings for ROCKPi 4B which is similar to 4A with > > > additional AP6256 Wifi/BT, PoE. > > > > > > Signed-off-by: Jagan Teki > > > --- > > > Changes for v2: > > > - new patch > > > > > > Documentation/devicetree/bindings/arm/rockchip.yaml | 6 ++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/arm/rockchip.yaml > > > b/Documentation/devicetree/bindings/arm/rockchip.yaml > > > index 36057c9e4b83..7250adb43d24 100644 > > > --- a/Documentation/devicetree/bindings/arm/rockchip.yaml > > > +++ b/Documentation/devicetree/bindings/arm/rockchip.yaml > > > @@ -448,6 +448,12 @@ properties: > > >- const: radxa,rockpi4a > > >- const: rockchip,rk3399 > > > > > > + - description: Radxa ROCK Pi 4B > > > +items: > > > + - const: radxa,rockpi4 > > > + - const: radxa,rockpi4b > > > + - const: rockchip,rk3399 > > > + > > > > Please do all RockPi4 variants into one entry, so we want something like: > > > > - description: Radxa ROCK Pi 4 > > What if the description has something like below. > > - description: Radxa ROCK Pi 4A/B/C That's also ok :-)
Re: [PATCH v2] drm/of: Consider the state in which the ep is disabled
Am Dienstag, 7. Juli 2020, 13:25:26 CEST schrieb Sandy Huang:
> don't mask possible_crtcs if remote-point is disabled.
>
> Signed-off-by: Sandy Huang
Reviewed-by: Heiko Stuebner
changes in v2:
- drop additional of_node_put, as ep will be put with the next
iteration of for_each_endpoint_of_node()
As this touches a pretty central function is there something
to keep in mind in regards to other DRM drivers?
[question for the broader audience ;-) ]
Heiko
> ---
> drivers/gpu/drm/drm_of.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
> index fdb05fbf72a0..565f05f5f11b 100644
> --- a/drivers/gpu/drm/drm_of.c
> +++ b/drivers/gpu/drm/drm_of.c
> @@ -66,6 +66,9 @@ uint32_t drm_of_find_possible_crtcs(struct drm_device *dev,
> uint32_t possible_crtcs = 0;
>
> for_each_endpoint_of_node(port, ep) {
> + if (!of_device_is_available(ep))
> + continue;
> +
> remote_port = of_graph_get_remote_port(ep);
> if (!remote_port) {
> of_node_put(ep);
>
Re: [PATCH] clk: rockchip: add CLK_IGNORE_UNUSED to RK3188 sclk_mac_lbtest
Hi,
Am Mittwoch, 22. Juli 2020, 16:31:37 CEST schrieb Alex Bee:
> Since the loopbacktest clock is not exported and is not touched in the
> driver, it needs the CLK_IGNORE_UNUSED flag in order to get the emac
> working.
could you please add it to the rk3188_critical_clocks array instead.
CLK_IGNORE_UNUSED only protects it against the clock subsystem
disabling it on boot, while as critical clock it also gets protected later.
Thanks
Heiko
>
> Signed-off-by: Alex Bee
> ---
> drivers/clk/rockchip/clk-rk3188.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clk/rockchip/clk-rk3188.c
> b/drivers/clk/rockchip/clk-rk3188.c
> index 77aebfb1d6d5..892b1edc3444 100644
> --- a/drivers/clk/rockchip/clk-rk3188.c
> +++ b/drivers/clk/rockchip/clk-rk3188.c
> @@ -354,7 +354,7 @@ static struct rockchip_clk_branch common_clk_branches[]
> __initdata = {
> RK2928_CLKGATE_CON(2), 5, GFLAGS),
> MUX(SCLK_MAC, "sclk_macref", mux_sclk_macref_p, CLK_SET_RATE_PARENT,
> RK2928_CLKSEL_CON(21), 4, 1, MFLAGS),
> - GATE(0, "sclk_mac_lbtest", "sclk_macref", 0,
> + GATE(0, "sclk_mac_lbtest", "sclk_macref", CLK_IGNORE_UNUSED,
> RK2928_CLKGATE_CON(2), 12, GFLAGS),
>
> COMPOSITE(0, "hsadc_src", mux_pll_src_gpll_cpll_p, 0,
>
Re: [PATCH v1 3/3] spi: rockchip: Fix error in SPI slave pio read
Hi Jon,
Am Mittwoch, 22. Juli 2020, 08:52:57 CEST schrieb Jon Lin:
> The RXFLR is possible larger than rx_left in Rockchip SPI, fix it.
>
> Signed-off-by: Jon Lin
> ---
> drivers/spi/spi-rockchip.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
> index a451dacab5cf..1f5e613b67d9 100644
> --- a/drivers/spi/spi-rockchip.c
> +++ b/drivers/spi/spi-rockchip.c
> @@ -291,7 +291,7 @@ static void rockchip_spi_pio_writer(struct rockchip_spi
> *rs)
> static void rockchip_spi_pio_reader(struct rockchip_spi *rs)
> {
> u32 words = readl_relaxed(rs->regs + ROCKCHIP_SPI_RXFLR);
> - u32 rx_left = rs->rx_left - words;
> + u32 rx_left = rs->rx_left > words ? rs->rx_left - words : 0;
I guess for future readability of the code braces might be nice, like
u32 rx_left = (rs->rx_left > words) ? rs->rx_left - words : 0;
But I stumbled onto (and fixed similarly) that issue yesterday as well, so
Reviewed-by: Heiko Stuebner
Heiko
Re: [PATCH v4 2/2] i2c: busses: convert to devm_platform_request_irq()
Am Freitag, 17. Juli 2020, 18:11:58 CEST schrieb Dejin Zheng:
> Use devm_platform_request_irq() to simplify code, and it contains
> platform_get_irq() and devm_request_irq().
>
> Cc: Michal Simek
> Cc: Wolfram Sang
> Signed-off-by: Dejin Zheng
> Acked-by: Linus Walleij
> Acked-by: Michal Simek
Rockchip part (i2c-rk3x):
Reviewed-by: Heiko Stuebner
> diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
> index 8e3cc85d1921..1f0ac69c5774 100644
> --- a/drivers/i2c/busses/i2c-rk3x.c
> +++ b/drivers/i2c/busses/i2c-rk3x.c
> @@ -1227,7 +1227,6 @@ static int rk3x_i2c_probe(struct platform_device *pdev)
> int ret = 0;
> int bus_nr;
> u32 value;
> - int irq;
> unsigned long clk_rate;
>
> i2c = devm_kzalloc(>dev, sizeof(struct rk3x_i2c), GFP_KERNEL);
> @@ -1289,17 +1288,10 @@ static int rk3x_i2c_probe(struct platform_device
> *pdev)
> }
> }
>
> - /* IRQ setup */
> - irq = platform_get_irq(pdev, 0);
> - if (irq < 0)
> - return irq;
> -
> - ret = devm_request_irq(>dev, irq, rk3x_i2c_irq,
> -0, dev_name(>dev), i2c);
> - if (ret < 0) {
> - dev_err(>dev, "cannot request IRQ\n");
> + ret = devm_platform_request_irq(pdev, 0, NULL, rk3x_i2c_irq,
> + 0, dev_name(>dev), i2c);
> + if (ret < 0)
> return ret;
> - }
>
> platform_set_drvdata(pdev, i2c);
>
Re: [PATCH 30/30] iio: adc: rockchip_saradc: Demote Demote seemingly unintentional kerneldoc header
Am Freitag, 17. Juli 2020, 18:55:38 CEST schrieb Lee Jones:
> This is the only use of kerneldoc in the source file and no
> descriptions are provided.
>
> Fixes the following W=1 kernel build warning(s):
>
> drivers/iio/adc/rockchip_saradc.c:190: warning: Function parameter or member
> 'reset' not described in 'rockchip_saradc_reset_controller'
>
> Cc: Heiko Stuebner
> Cc: Philipp Zabel
> Cc: linux-rockc...@lists.infradead.org
> Signed-off-by: Lee Jones
Subject-line says "Demote Demote..."
Otherwise
Reviewed-by: Heiko Stuebner
> ---
> drivers/iio/adc/rockchip_saradc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/rockchip_saradc.c
> b/drivers/iio/adc/rockchip_saradc.c
> index 582ba047c4a67..cf4ec59c1dab0 100644
> --- a/drivers/iio/adc/rockchip_saradc.c
> +++ b/drivers/iio/adc/rockchip_saradc.c
> @@ -183,7 +183,7 @@ static const struct of_device_id rockchip_saradc_match[]
> = {
> };
> MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
>
> -/**
> +/*
> * Reset SARADC Controller.
> */
> static void rockchip_saradc_reset_controller(struct reset_control *reset)
>
Re: [PATCH v2] drm/of: Consider the state in which the ep is disabled
Am Dienstag, 7. Juli 2020, 13:25:26 CEST schrieb Sandy Huang:
> don't mask possible_crtcs if remote-point is disabled.
>
> Signed-off-by: Sandy Huang
Reviewed-by: Heiko Stuebner
I guess this could've benefitted from a "changelog":
changes in v2:
- drop additional of_node_put, as ep will be put with the next
iteration of for_each_endpoint_of_node()
> ---
> drivers/gpu/drm/drm_of.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
> index fdb05fbf72a0..565f05f5f11b 100644
> --- a/drivers/gpu/drm/drm_of.c
> +++ b/drivers/gpu/drm/drm_of.c
> @@ -66,6 +66,9 @@ uint32_t drm_of_find_possible_crtcs(struct drm_device *dev,
> uint32_t possible_crtcs = 0;
>
> for_each_endpoint_of_node(port, ep) {
> + if (!of_device_is_available(ep))
> + continue;
> +
> remote_port = of_graph_get_remote_port(ep);
> if (!remote_port) {
> of_node_put(ep);
>
Re: [PATCH] drm/of: Consider the state in which the ep is disabled
Hi Sandy,
Am Montag, 6. Juli 2020, 09:59:44 CEST schrieb Sandy Huang:
> don't mask possible_crtcs if remote-point is disabled.
>
> Signed-off-by: Sandy Huang
> ---
> drivers/gpu/drm/drm_of.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
> index fdb05fbf72a0..f5f250435add 100644
> --- a/drivers/gpu/drm/drm_of.c
> +++ b/drivers/gpu/drm/drm_of.c
> @@ -66,6 +66,11 @@ uint32_t drm_of_find_possible_crtcs(struct drm_device *dev,
> uint32_t possible_crtcs = 0;
>
> for_each_endpoint_of_node(port, ep) {
> + if (!of_device_is_available(ep)) {
> + of_node_put(ep);
You'd only need the of_node_put here, if you were exiting the loop.
With the "continue" below, the next iteration of for_each_endpoint_of_node
will do the put on "ep"
Heiko
> + continue;
> + }
> +
> remote_port = of_graph_get_remote_port(ep);
> if (!remote_port) {
> of_node_put(ep);
>
Re: [PATCH v9 00/11] Genericize DW MIPI DSI bridge and add i.MX 6 driver
Hi Adrian, Am Dienstag, 9. Juni 2020, 19:49:48 CEST schrieb Adrian Ratiu: > [Re-submitting to cc dri-devel, sorry about the noise] > > Hello all, > > v9 cleanly applies on top of latest next-20200609 tree. at least it doesn't apply on top of current drm-misc-next for me which I really don't understand. Like patch 2/11 does @@ -31,6 +31,7 @@ #include . #define HWVER_131<><--><-->0x31333100<>/* IP version 1.31 */ +#define HWVER_130<><--><-->0x31333000<>/* IP version 1.30 */ . #define DSI_VERSION<--><--><-->0x00 #define VERSION<--><--><--><-->GENMASK(31, 8) where the file currently looks like #include #include #include #include #include #include #include #define HWVER_131 0x31333100 /* IP version 1.31 */ #define DSI_VERSION 0x00 #define VERSION GENMASK(31, 8) even in Linux-next So I guess ideally rebase on top of drm-misc-next Thanks Heiko
Re: [PATCH] PM / devfreq: rk3399_dmc: Fix kernel oops when rockchip,pmu is absent
Am Montag, 22. Juni 2020, 17:07:52 CEST schrieb Marc Zyngier: > Hi Heiko, > > On 2020-06-22 14:54, Heiko Stübner wrote: > > Hi Marc, > > > > Am Montag, 22. Juni 2020, 15:31:55 CEST schrieb Marc Zyngier: > >> On Sat, 13 Jun 2020 11:24:35 +0100 > >> Marc Zyngier wrote: > >> > >> > Booting a recent kernel on a rk3399-based system (nanopc-t4), > >> > equipped with a recent u-boot and ATF results in the following: > >> > > >> > [5.607431] Unable to handle kernel NULL pointer dereference at > >> > virtual address 01e4 > >> > [5.608219] Mem abort info: > >> > [5.608469] ESR = 0x9604 > >> > [5.608749] EC = 0x25: DABT (current EL), IL = 32 bits > >> > [5.609223] SET = 0, FnV = 0 > >> > [5.609600] EA = 0, S1PTW = 0 > >> > [5.609891] Data abort info: > >> > [5.610149] ISV = 0, ISS = 0x0004 > >> > [5.610489] CM = 0, WnR = 0 > >> > [5.610757] user pgtable: 4k pages, 48-bit VAs, pgdp=e62fb000 > >> > [5.611326] [01e4] pgd=, > >> > p4d= > >> > [5.611931] Internal error: Oops: 9604 [#1] SMP > >> > [5.612363] Modules linked in: rockchip_thermal(E+) rk3399_dmc(E+) > >> > soundcore(E) dw_wdt(E) rockchip_dfi(E) nvmem_rockchip_efuse(E) > >> > pwm_rockchip(E) cfg80211(E+) rockchip_saradc(E) industrialio(E) > >> > rfkill(E) cpufreq_dt(E) ip_tables(E) x_tables(E) autofs4(E) ext4(E) > >> > crc32c_generic(E) crc16(E) mbcache(E) jbd2(E) realtek(E) nvme(E) > >> > nvme_core(E) t10_pi(E) xhci_plat_hcd(E) xhci_hcd(E) rtc_rk808(E) > >> > rk808_regulator(E) clk_rk808(E) dwc3(E) udc_core(E) roles(E) ulpi(E) > >> > rk808(E) > >> fan53555(E) rockchipdrm(E) analogix_dp(E) dw_hdmi(E) cec(E) > >> dw_mipi_dsi(E) fixed(E) dwc3_of_simple(E) phy_rockchip_emmc(E) > >> gpio_keys(E) drm_kms_helper(E) phy_rockchip_inno_usb2(E) > >> ehci_platform(E) dwmac_rk(E) stmmac_platform(E) phy_rockchip_pcie(E) > >> ohci_platform(E) ohci_hcd(E) rockchip_io_domain(E) stmmac(E) > >> phy_rockchip_typec(E) ehci_hcd(E) sdhci_of_arasan(E) mdio_xpcs(E) > >> sdhci_pltfm(E) cqhci(E) drm(E) sdhci(E) phylink(E) of_mdio(E) > >> usbcore(E) i2c_rk3x(E) dw_mmc_rockchip(E) dw_mmc_pltfm(E) dw_mmc(E) > >> fixed_phy(E) libphy(E) > >> > [5.612454] pl330(E) > >> > [5.620255] CPU: 1 PID: 270 Comm: systemd-udevd Tainted: G > >> > E 5.7.0-13692-g83ae758d8b22 #1157 > >> > [5.621110] Hardware name: rockchip evb_rk3399/evb_rk3399, BIOS > >> > 2020.07-rc4-00023-g10d4cafe0f 06/10/2020 > >> > [5.621947] pstate: 4005 (nZcv daif -PAN -UAO BTYPE=--) > >> > [5.622446] pc : regmap_read+0x1c/0x80 > >> > [5.622787] lr : rk3399_dmcfreq_probe+0x6a4/0x8c0 [rk3399_dmc] > >> > [5.623299] sp : 8000126cb8a0 > >> > [5.623594] x29: 8000126cb8a0 x28: 8000126cbdb0 > >> > [5.624063] x27: f22dac40 x26: f6779800 > >> > [5.624533] x25: f6779810 x24: ffea > >> > [5.625002] x23: ffea x22: f65b74c8 > >> > [5.625471] x21: f783ca08 x20: f65b7480 > >> > [5.625941] x19: x18: 0001 > >> > [5.626410] x17: x16: > >> > [5.626878] x15: f22db138 x14: > >> > [5.627347] x13: 0018 x12: 80001106a8c7 > >> > [5.627817] x11: 0003 x10: 0101010101010101 > >> > [5.627861] systemd[1]: Found device SPCC M.2 PCIE SSD 3. > >> > [5.628286] x9 : 88d7c89c x8 : 7f7f7f7f7f7f7f7f > >> > [5.629238] x7 : fefefeff646c606d x6 : 1c0e0e0ee3e8e9f0 > >> > [5.629709] x5 : 706968630e0e0e1c x4 : 80808080 > >> > [5.630178] x3 : 937b1b5b1b434b80 x2 : 8000126cb944 > >> > [5.630648] x1 : 0308 x0 : > >> > [5.631119] Call trace: > >> > [5.631346] regmap_read+0x1c/0x80 > >> > [5.631654] rk3399_dmcfreq_probe+0x6a4/0x8c0 [rk3399_dmc] > >> > [5.632142] platform_drv_probe+0x5c/0xb0 > >> > [5.632500] really_probe+0xe4/0x448 > >> > [5.632819] driver_probe_device+0xfc/0x168 > >> > [5.633191] device_driver_attach+0
Re: [PATCH] PM / devfreq: rk3399_dmc: Fix kernel oops when rockchip,pmu is absent
Hi Marc, Am Montag, 22. Juni 2020, 15:31:55 CEST schrieb Marc Zyngier: > On Sat, 13 Jun 2020 11:24:35 +0100 > Marc Zyngier wrote: > > > Booting a recent kernel on a rk3399-based system (nanopc-t4), > > equipped with a recent u-boot and ATF results in the following: > > > > [5.607431] Unable to handle kernel NULL pointer dereference at virtual > > address 01e4 > > [5.608219] Mem abort info: > > [5.608469] ESR = 0x9604 > > [5.608749] EC = 0x25: DABT (current EL), IL = 32 bits > > [5.609223] SET = 0, FnV = 0 > > [5.609600] EA = 0, S1PTW = 0 > > [5.609891] Data abort info: > > [5.610149] ISV = 0, ISS = 0x0004 > > [5.610489] CM = 0, WnR = 0 > > [5.610757] user pgtable: 4k pages, 48-bit VAs, pgdp=e62fb000 > > [5.611326] [01e4] pgd=, p4d= > > [5.611931] Internal error: Oops: 9604 [#1] SMP > > [5.612363] Modules linked in: rockchip_thermal(E+) rk3399_dmc(E+) > > soundcore(E) dw_wdt(E) rockchip_dfi(E) nvmem_rockchip_efuse(E) > > pwm_rockchip(E) cfg80211(E+) rockchip_saradc(E) industrialio(E) rfkill(E) > > cpufreq_dt(E) ip_tables(E) x_tables(E) autofs4(E) ext4(E) crc32c_generic(E) > > crc16(E) mbcache(E) jbd2(E) realtek(E) nvme(E) nvme_core(E) t10_pi(E) > > xhci_plat_hcd(E) xhci_hcd(E) rtc_rk808(E) rk808_regulator(E) clk_rk808(E) > > dwc3(E) udc_core(E) roles(E) ulpi(E) rk808(E) > fan53555(E) rockchipdrm(E) analogix_dp(E) dw_hdmi(E) cec(E) > dw_mipi_dsi(E) fixed(E) dwc3_of_simple(E) phy_rockchip_emmc(E) > gpio_keys(E) drm_kms_helper(E) phy_rockchip_inno_usb2(E) > ehci_platform(E) dwmac_rk(E) stmmac_platform(E) phy_rockchip_pcie(E) > ohci_platform(E) ohci_hcd(E) rockchip_io_domain(E) stmmac(E) > phy_rockchip_typec(E) ehci_hcd(E) sdhci_of_arasan(E) mdio_xpcs(E) > sdhci_pltfm(E) cqhci(E) drm(E) sdhci(E) phylink(E) of_mdio(E) > usbcore(E) i2c_rk3x(E) dw_mmc_rockchip(E) dw_mmc_pltfm(E) dw_mmc(E) > fixed_phy(E) libphy(E) > > [5.612454] pl330(E) > > [5.620255] CPU: 1 PID: 270 Comm: systemd-udevd Tainted: GE > >5.7.0-13692-g83ae758d8b22 #1157 > > [5.621110] Hardware name: rockchip evb_rk3399/evb_rk3399, BIOS > > 2020.07-rc4-00023-g10d4cafe0f 06/10/2020 > > [5.621947] pstate: 4005 (nZcv daif -PAN -UAO BTYPE=--) > > [5.622446] pc : regmap_read+0x1c/0x80 > > [5.622787] lr : rk3399_dmcfreq_probe+0x6a4/0x8c0 [rk3399_dmc] > > [5.623299] sp : 8000126cb8a0 > > [5.623594] x29: 8000126cb8a0 x28: 8000126cbdb0 > > [5.624063] x27: f22dac40 x26: f6779800 > > [5.624533] x25: f6779810 x24: ffea > > [5.625002] x23: ffea x22: f65b74c8 > > [5.625471] x21: f783ca08 x20: f65b7480 > > [5.625941] x19: x18: 0001 > > [5.626410] x17: x16: > > [5.626878] x15: f22db138 x14: > > [5.627347] x13: 0018 x12: 80001106a8c7 > > [5.627817] x11: 0003 x10: 0101010101010101 > > [5.627861] systemd[1]: Found device SPCC M.2 PCIE SSD 3. > > [5.628286] x9 : 88d7c89c x8 : 7f7f7f7f7f7f7f7f > > [5.629238] x7 : fefefeff646c606d x6 : 1c0e0e0ee3e8e9f0 > > [5.629709] x5 : 706968630e0e0e1c x4 : 80808080 > > [5.630178] x3 : 937b1b5b1b434b80 x2 : 8000126cb944 > > [5.630648] x1 : 0308 x0 : > > [5.631119] Call trace: > > [5.631346] regmap_read+0x1c/0x80 > > [5.631654] rk3399_dmcfreq_probe+0x6a4/0x8c0 [rk3399_dmc] > > [5.632142] platform_drv_probe+0x5c/0xb0 > > [5.632500] really_probe+0xe4/0x448 > > [5.632819] driver_probe_device+0xfc/0x168 > > [5.633191] device_driver_attach+0x7c/0x88 > > [5.633567] __driver_attach+0xac/0x178 > > [5.633914] bus_for_each_dev+0x78/0xc8 > > [5.634261] driver_attach+0x2c/0x38 > > [5.634582] bus_add_driver+0x14c/0x230 > > [5.634925] driver_register+0x6c/0x128 > > [5.635269] __platform_driver_register+0x50/0x60 > > [5.635692] rk3399_dmcfreq_driver_init+0x2c/0x1000 [rk3399_dmc] > > [5.636226] do_one_initcall+0x50/0x230 > > [5.636569] do_init_module+0x60/0x248 > > [5.636902] load_module+0x21f8/0x28d8 > > [5.637237] __do_sys_finit_module+0xb0/0x118 > > [5.637627] __arm64_sys_finit_module+0x28/0x38 > > [5.638031] el0_svc_common.constprop.0+0x7c/0x1f8 > > [5.638456] do_el0_svc+0x2c/0x98 > > [5.638754] el0_svc+0x18/0x48 > > [5.639029] el0_sync_handler+0x8c/0x2d4 > > [5.639378] el0_sync+0x158/0x180 > > [5.639680] Code: a9bd7bfd 910003fd a90153f3 aa0003f3 (b941e400) > > [5.640221] ---[ end trace 63675fe5d0021970 ]--- > > > > This turns out to be due to the rk3399-dmc driver looking for > > an *undocumented* property (rockchip,pmu), and happily using > > a NULL pointer when the property isn't there. > > > > The very existence
Re: [PATCH v5 3/3] net: phy: mscc: handle the clkout control on some phy variants
Am Donnerstag, 18. Juni 2020, 18:40:15 CEST schrieb Russell King - ARM Linux admin: > On Thu, Jun 18, 2020 at 06:01:29PM +0200, Heiko Stübner wrote: > > Am Donnerstag, 18. Juni 2020, 17:47:48 CEST schrieb Russell King - ARM > > Linux admin: > > > On Thu, Jun 18, 2020 at 05:41:54PM +0200, Heiko Stübner wrote: > > > > Though I'm not sure how this fits in the whole bringup of ethernet phys. > > > > Like the phy is dependent on the underlying ethernet controller to > > > > actually turn it on. > > > > > > > > I guess we should check the phy-state and if it's not accessible, just > > > > keep the values and if it's in a suitable state do the configuration. > > > > > > > > Calling a vsc8531_config_clkout() from both the vsc8531_config_init() > > > > as well as the clk_(un-)prepare and clk_set_rate functions and being > > > > protected by a check against phy_is_started() ? > > > > > > It sounds like it doesn't actually fit the clk API paradym then. I > > > see that Rob suggested it, and from the DT point of view, it makes > > > complete sense, but then if the hardware can't actually be used in > > > the way the clk API expects it to be used, then there's a semantic > > > problem. > > > > > > What is this clock used for? > > > > It provides a source for the mac-clk for the actual transfers, here to > > provide the 125MHz clock needed for the RGMII interface . > > > > So right now the old rk3368-lion devicetree just declares a stub > > fixed-clock and instructs the soc's clock controller to use it [0] . > > And in the cover-letter here, I show the update variant with using > > the clock defined here. > > > > > > I've added the idea from my previous mail like shown below [1]. > > which would take into account the phy-state. > > > > But I guess I'll wait for more input before spamming people with v6. > > Let's get a handle on exactly what this is. > > The RGMII bus has two clocks: RXC and TXC, which are clocked at one > of 125MHz, 25MHz or 2.5MHz depending on the RGMII data rate. Some > PHYs replace TXC with GTX clock, which always runs at 125MHz. These > clocks are not what you're referring to. > > You are referring to another commonly provided clock between the MAC > and the PHY, something which is not unique to your PHY. > > We seem to be heading down a path where different PHYs end up doing > different things in DT for what is basically the same hardware setup, > which really isn't good. :( > > We have at803x using: > > qca,clk-out-frequency > qca,clk-out-strength > qca,keep-pll-enabled > > which are used to control the CLK_25M output pin on the device, which > may be used to provide a reference clock for the MAC side, selecting > between 25M, 50M, 62.5M and 125MHz. This was introduced in November > 2019, so not that long ago. Because it was not that old, was the reason I followed that example in my v1 :-) And Andrew then suggested to at least try to use a common property for the target frequency that other phys could migrate to. > Broadcom PHYs configure their 125MHz clock through the PHY device > flags passed from the MAC at attach/connect time. > > There's the dp83867 and dp83869 configuration (I'm not sure I can > make sense of it from reading the code) using ti,clk-output-sel - > but it looks like it's the same kind of thing. Introduced February > 2018 into one driver, and November 2019 in the other. > > It seems the Micrel PHYs produce a 125MHz clock irrespective of any > configuration (maybe configured by firmware, or hardware strapping?) > > So it seems we have four ways of doing the same thing today, and now > the suggestion is to implement a fifth different way. I think there > needs to be some consolidation here, maybe choosing one approach and > sticking with it. > > Hence, I disagree with Rob - we don't need a fifth approach, we need > to choose one approach and decide that's our policy for this and > apply it evenly across the board, rather than making up something > different each time a new PHY comes along. @Dave, @Andrew: what's you opinion here? As Russell above (and Florian in the binding patch) pointed out, integrating this into the clock-framework may prove difficult not only for consistency but also for dependency reasons. Personally I'm fine with either solution, I just want to achieve a working ethernet on my board :-D . Thanks Heiko
