Hi,
Thanks for your patch
On Sat, Nov 09, 2019 at 03:19:09PM +0000, Tian Yunhao wrote:
> The hws field of sun8i_v3s_hw_clks has only 74
> members. However, the number specified by CLK_NUMBER
> is 77 (= CLK_I2S0 + 1). This leads to runtime segmentation
> fault that is not always reproducible.
>
> This patch adds a protective field [CLK_NUMBER] which ensures
> ARRAY_SIZE(.hws) is always greater than .num, thus eliminates
> this error.
>
> Signed-off-by: Yunhao Tian <[email protected]>
> ---
> drivers/clk/sunxi-ng/ccu-sun8i-v3s.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c b/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c
> index 5c779eec454b..de7fce7f32e6 100644
> --- a/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c
> +++ b/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c
> @@ -617,6 +617,7 @@ static struct clk_hw_onecell_data sun8i_v3s_hw_clks = {
> [CLK_AVS] = &avs_clk.common.hw,
> [CLK_MBUS] = &mbus_clk.common.hw,
> [CLK_MIPI_CSI] = &mipi_csi_clk.common.hw,
> + [CLK_NUMBER] = NULL,
> },
> .num = CLK_NUMBER,
> };
> @@ -699,6 +700,7 @@ static struct clk_hw_onecell_data sun8i_v3_hw_clks = {
> [CLK_AVS] = &avs_clk.common.hw,
> [CLK_MBUS] = &mbus_clk.common.hw,
> [CLK_MIPI_CSI] = &mipi_csi_clk.common.hw,
> + [CLK_NUMBER] = NULL,
> },
> .num = CLK_NUMBER,
I'd rather have the number of clocks (.num) being properly set.
Maxime
于 2019年11月11日 GMT+08:00 下午8:39:36, Maxime Ripard <[email protected]> 写到:
>Hi,
>
>Thanks for your patch
>
>On Sat, Nov 09, 2019 at 03:19:09PM +0000, Tian Yunhao wrote:
>> The hws field of sun8i_v3s_hw_clks has only 74
>> members. However, the number specified by CLK_NUMBER
>> is 77 (= CLK_I2S0 + 1). This leads to runtime segmentation
>> fault that is not always reproducible.
>>
>> This patch adds a protective field [CLK_NUMBER] which ensures
>> ARRAY_SIZE(.hws) is always greater than .num, thus eliminates
>> this error.
>>
>> Signed-off-by: Yunhao Tian <[email protected]>
>> ---
>> drivers/clk/sunxi-ng/ccu-sun8i-v3s.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c
>b/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c
>> index 5c779eec454b..de7fce7f32e6 100644
>> --- a/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c
>> +++ b/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c
>> @@ -617,6 +617,7 @@ static struct clk_hw_onecell_data
>sun8i_v3s_hw_clks = {
>> [CLK_AVS] = &avs_clk.common.hw,
>> [CLK_MBUS] = &mbus_clk.common.hw,
>> [CLK_MIPI_CSI] = &mipi_csi_clk.common.hw,
>> + [CLK_NUMBER] = NULL,
>> },
>> .num = CLK_NUMBER,
>> };
>> @@ -699,6 +700,7 @@ static struct clk_hw_onecell_data
>sun8i_v3_hw_clks = {
>> [CLK_AVS] = &avs_clk.common.hw,
>> [CLK_MBUS] = &mbus_clk.common.hw,
>> [CLK_MIPI_CSI] = &mipi_csi_clk.common.hw,
>> + [CLK_NUMBER] = NULL,
>> },
>> .num = CLK_NUMBER,
>
>I'd rather have the number of clocks (.num) being properly set.
However the maximum clock indices number is different on V3s and V3, because
on V3s the last clock is missing.
Should we define CLK_NUMBER_V3S here?
>
>Maxime
On Tue, Nov 12, 2019 at 08:59:56PM +0800, Icenowy Zheng wrote:
>
>
> 于 2019年11月11日 GMT+08:00 下午8:39:36, Maxime Ripard <[email protected]> 写到:
> >Hi,
> >
> >Thanks for your patch
> >
> >On Sat, Nov 09, 2019 at 03:19:09PM +0000, Tian Yunhao wrote:
> >> The hws field of sun8i_v3s_hw_clks has only 74
> >> members. However, the number specified by CLK_NUMBER
> >> is 77 (= CLK_I2S0 + 1). This leads to runtime segmentation
> >> fault that is not always reproducible.
> >>
> >> This patch adds a protective field [CLK_NUMBER] which ensures
> >> ARRAY_SIZE(.hws) is always greater than .num, thus eliminates
> >> this error.
> >>
> >> Signed-off-by: Yunhao Tian <[email protected]>
> >> ---
> >> drivers/clk/sunxi-ng/ccu-sun8i-v3s.c | 2 ++
> >> 1 file changed, 2 insertions(+)
> >>
> >> diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c
> >b/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c
> >> index 5c779eec454b..de7fce7f32e6 100644
> >> --- a/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c
> >> +++ b/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c
> >> @@ -617,6 +617,7 @@ static struct clk_hw_onecell_data
> >sun8i_v3s_hw_clks = {
> >> [CLK_AVS] = &avs_clk.common.hw,
> >> [CLK_MBUS] = &mbus_clk.common.hw,
> >> [CLK_MIPI_CSI] = &mipi_csi_clk.common.hw,
> >> + [CLK_NUMBER] = NULL,
> >> },
> >> .num = CLK_NUMBER,
> >> };
> >> @@ -699,6 +700,7 @@ static struct clk_hw_onecell_data
> >sun8i_v3_hw_clks = {
> >> [CLK_AVS] = &avs_clk.common.hw,
> >> [CLK_MBUS] = &mbus_clk.common.hw,
> >> [CLK_MIPI_CSI] = &mipi_csi_clk.common.hw,
> >> + [CLK_NUMBER] = NULL,
> >> },
> >> .num = CLK_NUMBER,
> >
> >I'd rather have the number of clocks (.num) being properly set.
>
> However the maximum clock indices number is different on V3s and V3, because
> on V3s the last clock is missing.
>
> Should we define CLK_NUMBER_V3S here?
That, or we can just reference the last clock, we're not using
CLK_NUMBER anywhere else.
Maxime