| Message ID | IA1PR20MB49532E1A3D8BA71FDBB444BCBB85A@IA1PR20MB4953.namprd20.prod.outlook.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | riscv: sophgo: add clock support for Sophgo CV1800 SoCs | expand |
Inochi Amaoto wrote: > Add clock controller support for the Sophgo CV1800B and CV1812H. > > This patch follow this patch series: > https://lore.kernel.org/all/IA1PR20MB495399CAF2EEECC206ADA7ABBBD5A@IA1PR20MB4953.namprd20.prod.outlook.com/ > > Changed from v1: > 1. fix license issues. > > Inochi Amaoto (4): > dt-bindings: clock: sophgo: Add clock controller of CV1800 series SoC > clk: sophgo: Add CV1800 series clock controller driver > riscv: dts: sophgo: add clock generator for Sophgo CV1800 series SoC > riscv: dts: sophgo: add uart clock for Sophgo CV1800 series SoC Hi Inochi, This series seems to be missing patch 1 and 2. If you did send them, but just omitted linux-riscv from those patches, please don't do that. Having the whole series makes it a lot easier to review without having to hunt down all the missing parts on lore.kernel.org. scripts/get_maintainer.pl does support muliple patches as input /Emil
>Inochi Amaoto wrote: >> Add clock controller support for the Sophgo CV1800B and CV1812H. >> >> This patch follow this patch series: >> https://lore.kernel.org/all/IA1PR20MB495399CAF2EEECC206ADA7ABBBD5A@IA1PR20MB4953.namprd20.prod.outlook.com/ >> >> Changed from v1: >> 1. fix license issues. >> >> Inochi Amaoto (4): >> dt-bindings: clock: sophgo: Add clock controller of CV1800 series SoC >> clk: sophgo: Add CV1800 series clock controller driver >> riscv: dts: sophgo: add clock generator for Sophgo CV1800 series SoC >> riscv: dts: sophgo: add uart clock for Sophgo CV1800 series SoC > >Hi Inochi, > >This series seems to be missing patch 1 and 2. If you did send them, but just >omitted linux-riscv from those patches, please don't do that. Having the whole >series makes it a lot easier to review without having to hunt down all the >missing parts on lore.kernel.org. > >scripts/get_maintainer.pl does support muliple patches as input > >/Emil > Hi Emil, The get_maintainer.pl does not give me linux-riscv mail list for the first and second patch. I have added this to the second one, but the patch is held by the mail list since is too big. Anyway, I will add this mail list manually if you need. Sorry for this inconvenience. Thanks, Inochi
On 2023/12/5 19:55, Inochi Amaoto wrote: > Add driver for CV1800 series clock controller. Add more clarification on your changes. Seems you add several files with different names for different products, what's your design idea, please add some brief introduction. > > Signed-off-by: Inochi Amaoto <inochiama@outlook.com> > Link: https://github.com/milkv-duo/duo-files/blob/main/hardware/CV1800B/CV180X-Clock-v1.xlsx > Link: https://github.com/milkv-duo/duo-files/blob/main/hardware/CV1800B/CV1800B-CV1801B-Preliminary-Datasheet-full-en.pdf > --- > ...... > diff --git a/drivers/clk/sophgo/Kconfig b/drivers/clk/sophgo/Kconfig > new file mode 100644 > index 000000000000..243d58a30117 > --- /dev/null > +++ b/drivers/clk/sophgo/Kconfig > @@ -0,0 +1,7 @@ > +# SPDX-License-Identifier: GPL-2.0 > +# common clock support for SOPHGO SoC family. Drop this comment line, moving forward, this Kconfig file will be re-used for more different sophgo products. > + > +config CLK_SOPHGO_CV1800 > + tristate "Support for the Sophgo CV1800 series SoCs clock controller" > + default y > + depends on ARCH_SOPHGO || COMPILE_TEST Suggest to add some help words for this config item. ...... 2.43.0
> >On 2023/12/5 19:55, Inochi Amaoto wrote: >> Add driver for CV1800 series clock controller. >Add more clarification on your changes. Seems you add several files with different names for different products, what's your design idea, please add some brief introduction. In fact, it just adds the driver for the whole CV18XX series. I do not think its clock controller has something different for different product. The CV181X does have more clock, but it shares the same driver code of CV180X. All the things just follow the manual and are for the hardware design. Anyway, I will have a try. >> >> Signed-off-by: Inochi Amaoto <inochiama@outlook.com> >> Link: https://github.com/milkv-duo/duo-files/blob/main/hardware/CV1800B/CV180X-Clock-v1.xlsx >> Link: https://github.com/milkv-duo/duo-files/blob/main/hardware/CV1800B/CV1800B-CV1801B-Preliminary-Datasheet-full-en.pdf >> --- >> ...... >> diff --git a/drivers/clk/sophgo/Kconfig b/drivers/clk/sophgo/Kconfig >> new file mode 100644 >> index 000000000000..243d58a30117 >> --- /dev/null >> +++ b/drivers/clk/sophgo/Kconfig >> @@ -0,0 +1,7 @@ >> +# SPDX-License-Identifier: GPL-2.0 >> +# common clock support for SOPHGO SoC family. >Drop this comment line, moving forward, this Kconfig file will be re-used for more different sophgo products. Why? I do not think this have some impact on reuse. >> + >> +config CLK_SOPHGO_CV1800 >> + tristate "Support for the Sophgo CV1800 series SoCs clock controller" >> + default y >> + depends on ARCH_SOPHGO || COMPILE_TEST >Suggest to add some help words for this config item. > There is no extra information other than this title. In fact, I think the description title is enough to describe this. Add a duplicate help is useless. >...... > >2.43.0 > >
Inochi Amaoto wrote: > >Inochi Amaoto wrote: > >> Add clock controller support for the Sophgo CV1800B and CV1812H. > >> > >> This patch follow this patch series: > >> https://lore.kernel.org/all/IA1PR20MB495399CAF2EEECC206ADA7ABBBD5A@IA1PR20MB4953.namprd20.prod.outlook.com/ > >> > >> Changed from v1: > >> 1. fix license issues. > >> > >> Inochi Amaoto (4): > >> dt-bindings: clock: sophgo: Add clock controller of CV1800 series SoC > >> clk: sophgo: Add CV1800 series clock controller driver > >> riscv: dts: sophgo: add clock generator for Sophgo CV1800 series SoC > >> riscv: dts: sophgo: add uart clock for Sophgo CV1800 series SoC > > > >Hi Inochi, > > > >This series seems to be missing patch 1 and 2. If you did send them, but just > >omitted linux-riscv from those patches, please don't do that. Having the whole > >series makes it a lot easier to review without having to hunt down all the > >missing parts on lore.kernel.org. > > > >scripts/get_maintainer.pl does support muliple patches as input > > > >/Emil > > > > Hi Emil, > > The get_maintainer.pl does not give me linux-riscv mail list for the first > and second patch. I have added this to the second one, but the patch is > held by the mail list since is too big. Anyway, I will add this mail list > manually if you need. Sorry for this inconvenience. No worries. Yeah, that's what I meant by get_maintainer.pl supporting multiple patches. You can do something like git format-patch <starting point>.. ./scripts/get_maintainer.pl *.patch ..to get a list of recipients for the whole series. /Emil
Inochi Amaoto wrote: > > > >On 2023/12/5 19:55, Inochi Amaoto wrote: > >> Add driver for CV1800 series clock controller. > >Add more clarification on your changes. Seems you add several files with different names for different products, what's your design idea, please add some brief introduction. > > In fact, it just adds the driver for the whole CV18XX series. I do not > think its clock controller has something different for different product. > The CV181X does have more clock, but it shares the same driver code of > CV180X. All the things just follow the manual and are for the hardware > design. Anyway, I will have a try. > > >> > >> Signed-off-by: Inochi Amaoto <inochiama@outlook.com> > >> Link: https://github.com/milkv-duo/duo-files/blob/main/hardware/CV1800B/CV180X-Clock-v1.xlsx > >> Link: https://github.com/milkv-duo/duo-files/blob/main/hardware/CV1800B/CV1800B-CV1801B-Preliminary-Datasheet-full-en.pdf > >> --- > >> ...... > >> diff --git a/drivers/clk/sophgo/Kconfig b/drivers/clk/sophgo/Kconfig > >> new file mode 100644 > >> index 000000000000..243d58a30117 > >> --- /dev/null > >> +++ b/drivers/clk/sophgo/Kconfig > >> @@ -0,0 +1,7 @@ > >> +# SPDX-License-Identifier: GPL-2.0 > >> +# common clock support for SOPHGO SoC family. > >Drop this comment line, moving forward, this Kconfig file will be re-used for more different sophgo products. > > Why? I do not think this have some impact on reuse. > > >> + > >> +config CLK_SOPHGO_CV1800 > >> + tristate "Support for the Sophgo CV1800 series SoCs clock controller" > >> + default y > >> + depends on ARCH_SOPHGO || COMPILE_TEST > >Suggest to add some help words for this config item. > > > > There is no extra information other than this title. > In fact, I think the description title is enough to describe this. Add > a duplicate help is useless. I'd also like to see some more information here. Eg. what are examples of SoC's in the CV1800 series. checkpatch also complains: WARNING: please write a help paragraph that fully describes the config symbol #337: FILE: drivers/clk/sophgo/Kconfig:4: +config CLK_SOPHGO_CV1800 + tristate "Support for the Sophgo CV1800 series SoCs clock controller" + default y + depends on ARCH_SOPHGO || COMPILE_TEST Also the driver says "tristate" here, but defaults to built-in. If it works as a module it should be default m to not waste memory on systems not needing this. If it can't work properly as a module then tristate should be bool and the driver should be updated for that. /Emil
> > >Inochi Amaoto wrote: >>> >>> On 2023/12/5 19:55, Inochi Amaoto wrote: >>>> Add driver for CV1800 series clock controller. >>> Add more clarification on your changes. Seems you add several files with different names for different products, what's your design idea, please add some brief introduction. >> >> In fact, it just adds the driver for the whole CV18XX series. I do not >> think its clock controller has something different for different product. >> The CV181X does have more clock, but it shares the same driver code of >> CV180X. All the things just follow the manual and are for the hardware >> design. Anyway, I will have a try. >> >>>> >>>> Signed-off-by: Inochi Amaoto <inochiama@outlook.com> >>>> Link: https://github.com/milkv-duo/duo-files/blob/main/hardware/CV1800B/CV180X-Clock-v1.xlsx >>>> Link: https://github.com/milkv-duo/duo-files/blob/main/hardware/CV1800B/CV1800B-CV1801B-Preliminary-Datasheet-full-en.pdf >>>> --- >>>> ...... >>>> diff --git a/drivers/clk/sophgo/Kconfig b/drivers/clk/sophgo/Kconfig >>>> new file mode 100644 >>>> index 000000000000..243d58a30117 >>>> --- /dev/null >>>> +++ b/drivers/clk/sophgo/Kconfig >>>> @@ -0,0 +1,7 @@ >>>> +# SPDX-License-Identifier: GPL-2.0 >>>> +# common clock support for SOPHGO SoC family. >>> Drop this comment line, moving forward, this Kconfig file will be re-used for more different sophgo products. >> >> Why? I do not think this have some impact on reuse. >> >>>> + >>>> +config CLK_SOPHGO_CV1800 >>>> + tristate "Support for the Sophgo CV1800 series SoCs clock controller" >>>> + default y >>>> + depends on ARCH_SOPHGO || COMPILE_TEST >>> Suggest to add some help words for this config item. >>> >> >> There is no extra information other than this title. >> In fact, I think the description title is enough to describe this. Add >> a duplicate help is useless. > >I'd also like to see some more information here. Eg. what are examples of SoC's >in the CV1800 series. checkpatch also complains: > >WARNING: please write a help paragraph that fully describes the config symbol >#337: FILE: drivers/clk/sophgo/Kconfig:4: >+config CLK_SOPHGO_CV1800 >+ tristate "Support for the Sophgo CV1800 series SoCs clock controller" >+ default y >+ depends on ARCH_SOPHGO || COMPILE_TEST > OK, thanks for your advice, I will add this help. > >Also the driver says "tristate" here, but defaults to built-in. If it works as >a module it should be default m to not waste memory on systems not needing this. >If it can't work properly as a module then tristate should be bool and the >driver should be updated for that. > >/Emil > I will have a try and determine the right value. Thanks.
>Inochi Amaoto wrote: >>> Inochi Amaoto wrote: >>>> Add clock controller support for the Sophgo CV1800B and CV1812H. >>>> >>>> This patch follow this patch series: >>>> https://lore.kernel.org/all/IA1PR20MB495399CAF2EEECC206ADA7ABBBD5A@IA1PR20MB4953.namprd20.prod.outlook.com/ >>>> >>>> Changed from v1: >>>> 1. fix license issues. >>>> >>>> Inochi Amaoto (4): >>>> dt-bindings: clock: sophgo: Add clock controller of CV1800 series SoC >>>> clk: sophgo: Add CV1800 series clock controller driver >>>> riscv: dts: sophgo: add clock generator for Sophgo CV1800 series SoC >>>> riscv: dts: sophgo: add uart clock for Sophgo CV1800 series SoC >>> >>> Hi Inochi, >>> >>> This series seems to be missing patch 1 and 2. If you did send them, but just >>> omitted linux-riscv from those patches, please don't do that. Having the whole >>> series makes it a lot easier to review without having to hunt down all the >>> missing parts on lore.kernel.org. >>> >>> scripts/get_maintainer.pl does support muliple patches as input >>> >>> /Emil >>> >> >> Hi Emil, >> >> The get_maintainer.pl does not give me linux-riscv mail list for the first >> and second patch. I have added this to the second one, but the patch is >> held by the mail list since is too big. Anyway, I will add this mail list >> manually if you need. Sorry for this inconvenience. > >No worries. Yeah, that's what I meant by get_maintainer.pl supporting multiple >patches. You can do something like > > git format-patch <starting point>.. > ./scripts/get_maintainer.pl *.patch > >..to get a list of recipients for the whole series. > >/Emil > I have known this. But I only do this for cover. Anyway, I will send this patch in the way your mentioned.