| Message ID | 20220509052937.42283-1-samuel@sholland.org |
|---|---|
| Headers | show |
| Series | clk: sunxi: Out-of-bounds access fix and driver cleanup | expand |
On Mon, 9 May 2022 00:29:30 -0500 Samuel Holland <samuel@sholland.org> wrote: Hi Samuel, > This series fixes an issue with out-of-bounds access to the gate array > (patches 1-2), uses the rearranged array size information to remove a > bunch of duplicate code (patches 3-4), and then simplifies how the reset > driver is bound (patches 5-7). > > The original motivation for these changes was adding a driver for the > legacy A31/A23/A33 PRCM binding (which I will send separately), and > trying to use OF_PLATDATA in SPL (which did not work out). But I think > at least some of the cleanup is worth applying on its own. > > Patch 4 is generally the same change I made between v1 and v2 of the > pinctrl series, using some #ifdefs to share a U_BOOT_DRIVER. It's not > quite as clean as the pinctrl case, because here the SoC-specific parts > are in different files, so all of the CCU descriptors have to be global. so I skimmed over the series and quite like it: the negative diffstat speaks for itself ;-) I need to go over each individual patch with more scrutiny (unless someone beats me to it), but assuming no issues, I am planning on taking this for the next merge window. You mention out-of-bounds accesses: did you actually encounter one with the old or new DTs, that would need fixing now? Cheers, Andre > Samuel Holland (7): > clk: sunxi: Store the array sizes in the CCU descriptor > clk: sunxi: Prevent out-of-bounds gate array access > reset: sunxi: Get the reset count from the CCU descriptor > clk: sunxi: Use a single driver for all variants > clk: sunxi: Convert driver private data to platform data > reset: sunxi: Convert driver private data to platform data > reset: sunxi: Reuse the platform data from the clock driver > > drivers/clk/sunxi/clk_a10.c | 27 +----- > drivers/clk/sunxi/clk_a10s.c | 27 +----- > drivers/clk/sunxi/clk_a23.c | 27 +----- > drivers/clk/sunxi/clk_a31.c | 25 +---- > drivers/clk/sunxi/clk_a31_r.c | 29 +----- > drivers/clk/sunxi/clk_a64.c | 25 +---- > drivers/clk/sunxi/clk_a80.c | 36 ++------ > drivers/clk/sunxi/clk_a83t.c | 25 +---- > drivers/clk/sunxi/clk_h3.c | 27 +----- > drivers/clk/sunxi/clk_h6.c | 25 +---- > drivers/clk/sunxi/clk_h616.c | 25 +---- > drivers/clk/sunxi/clk_h6_r.c | 27 +----- > drivers/clk/sunxi/clk_r40.c | 25 +---- > drivers/clk/sunxi/clk_sunxi.c | 168 ++++++++++++++++++++++++++++++---- > drivers/clk/sunxi/clk_v3s.c | 27 +----- > drivers/reset/reset-sunxi.c | 55 ++--------- > include/clk/sunxi.h | 21 +---- > 17 files changed, 208 insertions(+), 413 deletions(-) >
On 5/9/22 1:29 AM, Samuel Holland wrote: > This series fixes an issue with out-of-bounds access to the gate array > (patches 1-2), uses the rearranged array size information to remove a > bunch of duplicate code (patches 3-4), and then simplifies how the reset > driver is bound (patches 5-7). > > The original motivation for these changes was adding a driver for the > legacy A31/A23/A33 PRCM binding (which I will send separately), and > trying to use OF_PLATDATA in SPL (which did not work out). But I think > at least some of the cleanup is worth applying on its own. > > Patch 4 is generally the same change I made between v1 and v2 of the > pinctrl series, using some #ifdefs to share a U_BOOT_DRIVER. It's not > quite as clean as the pinctrl case, because here the SoC-specific parts > are in different files, so all of the CCU descriptors have to be global. > > > Samuel Holland (7): > clk: sunxi: Store the array sizes in the CCU descriptor > clk: sunxi: Prevent out-of-bounds gate array access > reset: sunxi: Get the reset count from the CCU descriptor > clk: sunxi: Use a single driver for all variants > clk: sunxi: Convert driver private data to platform data > reset: sunxi: Convert driver private data to platform data > reset: sunxi: Reuse the platform data from the clock driver > > drivers/clk/sunxi/clk_a10.c | 27 +----- > drivers/clk/sunxi/clk_a10s.c | 27 +----- > drivers/clk/sunxi/clk_a23.c | 27 +----- > drivers/clk/sunxi/clk_a31.c | 25 +---- > drivers/clk/sunxi/clk_a31_r.c | 29 +----- > drivers/clk/sunxi/clk_a64.c | 25 +---- > drivers/clk/sunxi/clk_a80.c | 36 ++------ > drivers/clk/sunxi/clk_a83t.c | 25 +---- > drivers/clk/sunxi/clk_h3.c | 27 +----- > drivers/clk/sunxi/clk_h6.c | 25 +---- > drivers/clk/sunxi/clk_h616.c | 25 +---- > drivers/clk/sunxi/clk_h6_r.c | 27 +----- > drivers/clk/sunxi/clk_r40.c | 25 +---- > drivers/clk/sunxi/clk_sunxi.c | 168 ++++++++++++++++++++++++++++++---- > drivers/clk/sunxi/clk_v3s.c | 27 +----- > drivers/reset/reset-sunxi.c | 55 ++--------- > include/clk/sunxi.h | 21 +---- > 17 files changed, 208 insertions(+), 413 deletions(-) > For this series: Acked-by: Sean Anderson <seanga2@gmail.com>
On Mon, 9 May 2022 00:29:30 -0500 Samuel Holland <samuel@sholland.org> wrote: Hi Samuel, > This series fixes an issue with out-of-bounds access to the gate array > (patches 1-2), uses the rearranged array size information to remove a > bunch of duplicate code (patches 3-4), and then simplifies how the reset > driver is bound (patches 5-7). > > The original motivation for these changes was adding a driver for the > legacy A31/A23/A33 PRCM binding (which I will send separately), and > trying to use OF_PLATDATA in SPL (which did not work out). But I think > at least some of the cleanup is worth applying on its own. > > Patch 4 is generally the same change I made between v1 and v2 of the > pinctrl series, using some #ifdefs to share a U_BOOT_DRIVER. It's not > quite as clean as the pinctrl case, because here the SoC-specific parts > are in different files, so all of the CCU descriptors have to be global. Many thanks for your work on this! So I am done with reviewing this series, and as mentioned quite like it and am fine with it. I added F1C100s support, which got added in U-Boot meanwhile, and pushed that to sunxi/next: https://source.denx.de/u-boot/custodians/u-boot-sunxi/-/commits/next/ Can you please have a look and confirm that this is fine? I will then send the PR in about two weeks time, if nothing breaks. Cheers, Andre > Samuel Holland (7): > clk: sunxi: Store the array sizes in the CCU descriptor > clk: sunxi: Prevent out-of-bounds gate array access > reset: sunxi: Get the reset count from the CCU descriptor > clk: sunxi: Use a single driver for all variants > clk: sunxi: Convert driver private data to platform data > reset: sunxi: Convert driver private data to platform data > reset: sunxi: Reuse the platform data from the clock driver > > drivers/clk/sunxi/clk_a10.c | 27 +----- > drivers/clk/sunxi/clk_a10s.c | 27 +----- > drivers/clk/sunxi/clk_a23.c | 27 +----- > drivers/clk/sunxi/clk_a31.c | 25 +---- > drivers/clk/sunxi/clk_a31_r.c | 29 +----- > drivers/clk/sunxi/clk_a64.c | 25 +---- > drivers/clk/sunxi/clk_a80.c | 36 ++------ > drivers/clk/sunxi/clk_a83t.c | 25 +---- > drivers/clk/sunxi/clk_h3.c | 27 +----- > drivers/clk/sunxi/clk_h6.c | 25 +---- > drivers/clk/sunxi/clk_h616.c | 25 +---- > drivers/clk/sunxi/clk_h6_r.c | 27 +----- > drivers/clk/sunxi/clk_r40.c | 25 +---- > drivers/clk/sunxi/clk_sunxi.c | 168 ++++++++++++++++++++++++++++++---- > drivers/clk/sunxi/clk_v3s.c | 27 +----- > drivers/reset/reset-sunxi.c | 55 ++--------- > include/clk/sunxi.h | 21 +---- > 17 files changed, 208 insertions(+), 413 deletions(-) >
Hi Andre, On 6/27/22 7:40 PM, Andre Przywara wrote: > On Mon, 9 May 2022 00:29:30 -0500 > Samuel Holland <samuel@sholland.org> wrote: > > Hi Samuel, > >> This series fixes an issue with out-of-bounds access to the gate array >> (patches 1-2), uses the rearranged array size information to remove a >> bunch of duplicate code (patches 3-4), and then simplifies how the reset >> driver is bound (patches 5-7). >> >> The original motivation for these changes was adding a driver for the >> legacy A31/A23/A33 PRCM binding (which I will send separately), and >> trying to use OF_PLATDATA in SPL (which did not work out). But I think >> at least some of the cleanup is worth applying on its own. >> >> Patch 4 is generally the same change I made between v1 and v2 of the >> pinctrl series, using some #ifdefs to share a U_BOOT_DRIVER. It's not >> quite as clean as the pinctrl case, because here the SoC-specific parts >> are in different files, so all of the CCU descriptors have to be global. > > Many thanks for your work on this! > So I am done with reviewing this series, and as mentioned quite like it > and am fine with it. I added F1C100s support, which got added in U-Boot > meanwhile, and pushed that to sunxi/next: > https://source.denx.de/u-boot/custodians/u-boot-sunxi/-/commits/next/ > > Can you please have a look and confirm that this is fine? I will then > send the PR in about two weeks time, if nothing breaks. Looks good to me, and thanks for taking care of F1C100s. My only trivial comment is that you left out Sean's ack. Regards, Samuel