Re: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support

oe-kbuild-all.lists.linux.dev archive mirror
 help / color / mirror / Atom feed

* Re: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support
       [not found] <20230307115022.12846-2-zhuyinbo@loongson.cn>
@ 2023-03-08 12:16 ` kernel test robot
  2023-03-09  2:58   ` zhuyinbo
  0 siblings, 1 reply; 13+ messages in thread
From: kernel test robot @ 2023-03-08 12:16 UTC (permalink / raw)
  To: Yinbo Zhu, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, linux-kernel, linux-clk, devicetree
  Cc: oe-kbuild-all, Jianmin Lv, Liu Peibao, wanghongliang,
	loongson-kernel, Yinbo Zhu

Hi Yinbo,

I love your patch! Yet something to improve:

[auto build test ERROR on clk/clk-next]
[also build test ERROR on robh/for-next linus/master v6.3-rc1 next-20230308]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Yinbo-Zhu/clk-clk-loongson2-add-clock-controller-driver-support/20230307-195252
base:   https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
patch link:    https://lore.kernel.org/r/20230307115022.12846-2-zhuyinbo%40loongson.cn
patch subject: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support
config: mips-allyesconfig (https://download.01.org/0day-ci/archive/20230308/202303082037.QPfBP64A-lkp@intel.com/config)
compiler: mips-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/391d6fc63ac65f5456e4755c9dd85232a6296285
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Yinbo-Zhu/clk-clk-loongson2-add-clock-controller-driver-support/20230307-195252
        git checkout 391d6fc63ac65f5456e4755c9dd85232a6296285
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=mips olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303082037.QPfBP64A-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/clk/clk-loongson2.c: In function 'loongson2_calc_pll_rate':
>> drivers/clk/clk-loongson2.c:79:15: error: implicit declaration of function 'readq'; did you mean 'readl'? [-Werror=implicit-function-declaration]
      79 |         val = readq(loongson2_pll_base + offset);
         |               ^~~~~
         |               readl
   cc1: some warnings being treated as errors


vim +79 drivers/clk/clk-loongson2.c

    73	
    74	static unsigned long loongson2_calc_pll_rate(int offset, unsigned long rate)
    75	{
    76		u64 val;
    77		u32 mult = 1, div = 1;
    78	
  > 79		val = readq(loongson2_pll_base + offset);
    80	
    81		mult = (val >> LOONGSON2_PLL_MULT_SHIFT) &
    82				clk_div_mask(LOONGSON2_PLL_MULT_WIDTH);
    83		div = (val >> LOONGSON2_PLL_DIV_SHIFT) &
    84				clk_div_mask(LOONGSON2_PLL_DIV_WIDTH);
    85	
    86		return div_u64((u64)rate * mult, div);
    87	}
    88	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support
  2023-03-08 12:16 ` [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support kernel test robot
@ 2023-03-09  2:58   ` zhuyinbo
  2023-03-09  3:18     ` zhuyinbo
                       ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: zhuyinbo @ 2023-03-09  2:58 UTC (permalink / raw)
  To: kernel test robot, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, linux-kernel, linux-clk, devicetree
  Cc: oe-kbuild-all, Jianmin Lv, Liu Peibao, wanghongliang, loongson-kernel


在 2023/3/8 下午8:16, kernel test robot 写道:
> Hi Yinbo,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on clk/clk-next]
> [also build test ERROR on robh/for-next linus/master v6.3-rc1 next-20230308]
> [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#_base_tree_information]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/Yinbo-Zhu/clk-clk-loongson2-add-clock-controller-driver-support/20230307-195252
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
> patch link:    https://lore.kernel.org/r/20230307115022.12846-2-zhuyinbo%40loongson.cn
> patch subject: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support
> config: mips-allyesconfig (https://download.01.org/0day-ci/archive/20230308/202303082037.QPfBP64A-lkp@intel.com/config)
> compiler: mips-linux-gcc (GCC) 12.1.0
> reproduce (this is a W=1 build):
>          wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>          chmod +x ~/bin/make.cross
>          # https://github.com/intel-lab-lkp/linux/commit/391d6fc63ac65f5456e4755c9dd85232a6296285
>          git remote add linux-review https://github.com/intel-lab-lkp/linux
>          git fetch --no-tags linux-review Yinbo-Zhu/clk-clk-loongson2-add-clock-controller-driver-support/20230307-195252
>          git checkout 391d6fc63ac65f5456e4755c9dd85232a6296285
>          # save the config file
>          mkdir build_dir && cp config build_dir/.config
>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=mips olddefconfig
>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@intel.com>
> | Link: https://lore.kernel.org/oe-kbuild-all/202303082037.QPfBP64A-lkp@intel.com/
>
> All errors (new ones prefixed by >>):
>
>     drivers/clk/clk-loongson2.c: In function 'loongson2_calc_pll_rate':
>>> drivers/clk/clk-loongson2.c:79:15: error: implicit declaration of function 'readq'; did you mean 'readl'? [-Werror=implicit-function-declaration]
>        79 |         val = readq(loongson2_pll_base + offset);
>           |               ^~~~~
>           |               readl
>     cc1: some warnings being treated as errors

The CONFIG_64BIT not enabled in your config file, I will add a depend on 
"CONFIG_64BIT" in my clock driver to fix this compile error.

>
>
> vim +79 drivers/clk/clk-loongson2.c
>
>      73	
>      74	static unsigned long loongson2_calc_pll_rate(int offset, unsigned long rate)
>      75	{
>      76		u64 val;
>      77		u32 mult = 1, div = 1;
>      78	
>    > 79		val = readq(loongson2_pll_base + offset);
>      80	
>      81		mult = (val >> LOONGSON2_PLL_MULT_SHIFT) &
>      82				clk_div_mask(LOONGSON2_PLL_MULT_WIDTH);
>      83		div = (val >> LOONGSON2_PLL_DIV_SHIFT) &
>      84				clk_div_mask(LOONGSON2_PLL_DIV_WIDTH);
>      85	
>      86		return div_u64((u64)rate * mult, div);
>      87	}
>      88	
>


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support
  2023-03-09  2:58   ` zhuyinbo
@ 2023-03-09  3:18     ` zhuyinbo
  2023-03-09  6:09       ` Krzysztof Kozlowski
  2023-03-09  6:09     ` Krzysztof Kozlowski
  2023-03-09 23:47     ` Stephen Boyd
  2 siblings, 1 reply; 13+ messages in thread
From: zhuyinbo @ 2023-03-09  3:18 UTC (permalink / raw)
  To: kernel test robot, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, linux-kernel, linux-clk, devicetree
  Cc: oe-kbuild-all, Jianmin Lv, Liu Peibao, wanghongliang, loongson-kernel


在 2023/3/9 上午10:58, zhuyinbo 写道:
>
> 在 2023/3/8 下午8:16, kernel test robot 写道:
>> Hi Yinbo,
>>
>> I love your patch! Yet something to improve:
>>
>> [auto build test ERROR on clk/clk-next]
>> [also build test ERROR on robh/for-next linus/master v6.3-rc1 
>> next-20230308]
>> [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#_base_tree_information]
>>
>> url: 
>> https://github.com/intel-lab-lkp/linux/commits/Yinbo-Zhu/clk-clk-loongson2-add-clock-controller-driver-support/20230307-195252
>> base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git 
>> clk-next
>> patch link: 
>> https://lore.kernel.org/r/20230307115022.12846-2-zhuyinbo%40loongson.cn
>> patch subject: [PATCH v13 2/2] clk: clk-loongson2: add clock 
>> controller driver support
>> config: mips-allyesconfig 
>> (https://download.01.org/0day-ci/archive/20230308/202303082037.QPfBP64A-lkp@intel.com/config)
>> compiler: mips-linux-gcc (GCC) 12.1.0
>> reproduce (this is a W=1 build):
>>          wget 
>> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross 
>> -O ~/bin/make.cross
>>          chmod +x ~/bin/make.cross
>>          # 
>> https://github.com/intel-lab-lkp/linux/commit/391d6fc63ac65f5456e4755c9dd85232a6296285
>>          git remote add linux-review 
>> https://github.com/intel-lab-lkp/linux
>>          git fetch --no-tags linux-review 
>> Yinbo-Zhu/clk-clk-loongson2-add-clock-controller-driver-support/20230307-195252
>>          git checkout 391d6fc63ac65f5456e4755c9dd85232a6296285
>>          # save the config file
>>          mkdir build_dir && cp config build_dir/.config
>>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 
>> make.cross W=1 O=build_dir ARCH=mips olddefconfig
>>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 
>> make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/
>>
>> If you fix the issue, kindly add following tag where applicable
>> | Reported-by: kernel test robot <lkp@intel.com>
>> | Link: 
>> https://lore.kernel.org/oe-kbuild-all/202303082037.QPfBP64A-lkp@intel.com/
>>
>> All errors (new ones prefixed by >>):
>>
>>     drivers/clk/clk-loongson2.c: In function 'loongson2_calc_pll_rate':
>>>> drivers/clk/clk-loongson2.c:79:15: error: implicit declaration of 
>>>> function 'readq'; did you mean 'readl'? 
>>>> [-Werror=implicit-function-declaration]
>>        79 |         val = readq(loongson2_pll_base + offset);
>>           |               ^~~~~
>>           |               readl
>>     cc1: some warnings being treated as errors
>
> The CONFIG_64BIT not enabled in your config file, I will add a depend 
> on "CONFIG_64BIT" in my clock driver to fix this compile error.
My clock is for LoongArch platform, The LOONGARCH had select 
"CONFIG_64BIT", I will add a depend on "LOONGARCH" in my clock driver to 
fix this compile error.
>
>>
>>
>> vim +79 drivers/clk/clk-loongson2.c
>>
>>      73
>>      74    static unsigned long loongson2_calc_pll_rate(int offset, 
>> unsigned long rate)
>>      75    {
>>      76        u64 val;
>>      77        u32 mult = 1, div = 1;
>>      78
>>    > 79        val = readq(loongson2_pll_base + offset);
>>      80
>>      81        mult = (val >> LOONGSON2_PLL_MULT_SHIFT) &
>>      82                clk_div_mask(LOONGSON2_PLL_MULT_WIDTH);
>>      83        div = (val >> LOONGSON2_PLL_DIV_SHIFT) &
>>      84                clk_div_mask(LOONGSON2_PLL_DIV_WIDTH);
>>      85
>>      86        return div_u64((u64)rate * mult, div);
>>      87    }
>>      88
>>


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support
  2023-03-09  3:18     ` zhuyinbo
@ 2023-03-09  6:09       ` Krzysztof Kozlowski
  2023-03-09  6:27         ` zhuyinbo
  0 siblings, 1 reply; 13+ messages in thread
From: Krzysztof Kozlowski @ 2023-03-09  6:09 UTC (permalink / raw)
  To: zhuyinbo, kernel test robot, Michael Turquette, Stephen Boyd,
	Rob Herring, linux-kernel, linux-clk, devicetree
  Cc: oe-kbuild-all, Jianmin Lv, Liu Peibao, wanghongliang, loongson-kernel

On 09/03/2023 04:18, zhuyinbo wrote:
> 
> 在 2023/3/9 上午10:58, zhuyinbo 写道:
>>
>> 在 2023/3/8 下午8:16, kernel test robot 写道:
>>> Hi Yinbo,
>>>
>>> I love your patch! Yet something to improve:
>>>
>>> [auto build test ERROR on clk/clk-next]
>>> [also build test ERROR on robh/for-next linus/master v6.3-rc1 
>>> next-20230308]
>>> [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#_base_tree_information]
>>>
>>> url: 
>>> https://github.com/intel-lab-lkp/linux/commits/Yinbo-Zhu/clk-clk-loongson2-add-clock-controller-driver-support/20230307-195252
>>> base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git 
>>> clk-next
>>> patch link: 
>>> https://lore.kernel.org/r/20230307115022.12846-2-zhuyinbo%40loongson.cn
>>> patch subject: [PATCH v13 2/2] clk: clk-loongson2: add clock 
>>> controller driver support
>>> config: mips-allyesconfig 
>>> (https://download.01.org/0day-ci/archive/20230308/202303082037.QPfBP64A-lkp@intel.com/config)
>>> compiler: mips-linux-gcc (GCC) 12.1.0
>>> reproduce (this is a W=1 build):
>>>          wget 
>>> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross 
>>> -O ~/bin/make.cross
>>>          chmod +x ~/bin/make.cross
>>>          # 
>>> https://github.com/intel-lab-lkp/linux/commit/391d6fc63ac65f5456e4755c9dd85232a6296285
>>>          git remote add linux-review 
>>> https://github.com/intel-lab-lkp/linux
>>>          git fetch --no-tags linux-review 
>>> Yinbo-Zhu/clk-clk-loongson2-add-clock-controller-driver-support/20230307-195252
>>>          git checkout 391d6fc63ac65f5456e4755c9dd85232a6296285
>>>          # save the config file
>>>          mkdir build_dir && cp config build_dir/.config
>>>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 
>>> make.cross W=1 O=build_dir ARCH=mips olddefconfig
>>>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 
>>> make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/
>>>
>>> If you fix the issue, kindly add following tag where applicable
>>> | Reported-by: kernel test robot <lkp@intel.com>
>>> | Link: 
>>> https://lore.kernel.org/oe-kbuild-all/202303082037.QPfBP64A-lkp@intel.com/
>>>
>>> All errors (new ones prefixed by >>):
>>>
>>>     drivers/clk/clk-loongson2.c: In function 'loongson2_calc_pll_rate':
>>>>> drivers/clk/clk-loongson2.c:79:15: error: implicit declaration of 
>>>>> function 'readq'; did you mean 'readl'? 
>>>>> [-Werror=implicit-function-declaration]
>>>        79 |         val = readq(loongson2_pll_base + offset);
>>>           |               ^~~~~
>>>           |               readl
>>>     cc1: some warnings being treated as errors
>>
>> The CONFIG_64BIT not enabled in your config file, I will add a depend 
>> on "CONFIG_64BIT" in my clock driver to fix this compile error.
> My clock is for LoongArch platform, The LOONGARCH had select 
> "CONFIG_64BIT", I will add a depend on "LOONGARCH" in my clock driver to 
> fix this compile error.

No. Fix your code instead.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support
  2023-03-09  6:09       ` Krzysztof Kozlowski
@ 2023-03-09  6:27         ` zhuyinbo
  2023-03-09  6:37           ` Krzysztof Kozlowski
  0 siblings, 1 reply; 13+ messages in thread
From: zhuyinbo @ 2023-03-09  6:27 UTC (permalink / raw)
  To: Krzysztof Kozlowski, kernel test robot, Michael Turquette,
	zhuyinbo, Stephen Boyd, Rob Herring, linux-kernel, linux-clk,
	devicetree
  Cc: oe-kbuild-all, Jianmin Lv, Liu Peibao, wanghongliang, loongson-kernel


在 2023/3/9 下午2:09, Krzysztof Kozlowski 写道:
> On 09/03/2023 04:18, zhuyinbo wrote:
>> 在 2023/3/9 上午10:58, zhuyinbo 写道:
>>> 在 2023/3/8 下午8:16, kernel test robot 写道:
>>>> Hi Yinbo,
>>>>
>>>> I love your patch! Yet something to improve:
>>>>
>>>> [auto build test ERROR on clk/clk-next]
>>>> [also build test ERROR on robh/for-next linus/master v6.3-rc1
>>>> next-20230308]
>>>> [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#_base_tree_information]
>>>>
>>>> url:
>>>> https://github.com/intel-lab-lkp/linux/commits/Yinbo-Zhu/clk-clk-loongson2-add-clock-controller-driver-support/20230307-195252
>>>> base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git
>>>> clk-next
>>>> patch link:
>>>> https://lore.kernel.org/r/20230307115022.12846-2-zhuyinbo%40loongson.cn
>>>> patch subject: [PATCH v13 2/2] clk: clk-loongson2: add clock
>>>> controller driver support
>>>> config: mips-allyesconfig
>>>> (https://download.01.org/0day-ci/archive/20230308/202303082037.QPfBP64A-lkp@intel.com/config)
>>>> compiler: mips-linux-gcc (GCC) 12.1.0
>>>> reproduce (this is a W=1 build):
>>>>           wget
>>>> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
>>>> -O ~/bin/make.cross
>>>>           chmod +x ~/bin/make.cross
>>>>           #
>>>> https://github.com/intel-lab-lkp/linux/commit/391d6fc63ac65f5456e4755c9dd85232a6296285
>>>>           git remote add linux-review
>>>> https://github.com/intel-lab-lkp/linux
>>>>           git fetch --no-tags linux-review
>>>> Yinbo-Zhu/clk-clk-loongson2-add-clock-controller-driver-support/20230307-195252
>>>>           git checkout 391d6fc63ac65f5456e4755c9dd85232a6296285
>>>>           # save the config file
>>>>           mkdir build_dir && cp config build_dir/.config
>>>>           COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0
>>>> make.cross W=1 O=build_dir ARCH=mips olddefconfig
>>>>           COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0
>>>> make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/
>>>>
>>>> If you fix the issue, kindly add following tag where applicable
>>>> | Reported-by: kernel test robot <lkp@intel.com>
>>>> | Link:
>>>> https://lore.kernel.org/oe-kbuild-all/202303082037.QPfBP64A-lkp@intel.com/
>>>>
>>>> All errors (new ones prefixed by >>):
>>>>
>>>>      drivers/clk/clk-loongson2.c: In function 'loongson2_calc_pll_rate':
>>>>>> drivers/clk/clk-loongson2.c:79:15: error: implicit declaration of
>>>>>> function 'readq'; did you mean 'readl'?
>>>>>> [-Werror=implicit-function-declaration]
>>>>         79 |         val = readq(loongson2_pll_base + offset);
>>>>            |               ^~~~~
>>>>            |               readl
>>>>      cc1: some warnings being treated as errors
>>> The CONFIG_64BIT not enabled in your config file, I will add a depend
>>> on "CONFIG_64BIT" in my clock driver to fix this compile error.
>> My clock is for LoongArch platform, The LOONGARCH had select
>> "CONFIG_64BIT", I will add a depend on "LOONGARCH" in my clock driver to
>> fix this compile error.
> No. Fix your code instead.

but the readq that ask CONFIG_64BIT is enabled,  definition in 
include/asm-generic/io.h

so need enable CONFIG_64BIT for my clk driver.


#ifdef CONFIG_64BIT
#ifndef readq
#define readq readq
static inline u64 readq(const volatile void __iomem *addr)
{
         u64 val;

         __io_br();
         val = __le64_to_cpu(__raw_readq(addr));
         __io_ar();
         return val;
}
#endif
#endif /* CONFIG_64BIT */

>
> Best regards,
> Krzysztof


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support
  2023-03-09  6:27         ` zhuyinbo
@ 2023-03-09  6:37           ` Krzysztof Kozlowski
  0 siblings, 0 replies; 13+ messages in thread
From: Krzysztof Kozlowski @ 2023-03-09  6:37 UTC (permalink / raw)
  To: zhuyinbo, kernel test robot, Michael Turquette, Stephen Boyd,
	Rob Herring, linux-kernel, linux-clk, devicetree
  Cc: oe-kbuild-all, Jianmin Lv, Liu Peibao, wanghongliang, loongson-kernel

On 09/03/2023 07:27, zhuyinbo wrote:
> 
> 在 2023/3/9 下午2:09, Krzysztof Kozlowski 写道:
>> On 09/03/2023 04:18, zhuyinbo wrote:
>>> 在 2023/3/9 上午10:58, zhuyinbo 写道:
>>>> 在 2023/3/8 下午8:16, kernel test robot 写道:
>>>>> Hi Yinbo,
>>>>>
>>>>> I love your patch! Yet something to improve:
>>>>>
>>>>> [auto build test ERROR on clk/clk-next]
>>>>> [also build test ERROR on robh/for-next linus/master v6.3-rc1
>>>>> next-20230308]
>>>>> [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#_base_tree_information]
>>>>>
>>>>> url:
>>>>> https://github.com/intel-lab-lkp/linux/commits/Yinbo-Zhu/clk-clk-loongson2-add-clock-controller-driver-support/20230307-195252
>>>>> base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git
>>>>> clk-next
>>>>> patch link:
>>>>> https://lore.kernel.org/r/20230307115022.12846-2-zhuyinbo%40loongson.cn
>>>>> patch subject: [PATCH v13 2/2] clk: clk-loongson2: add clock
>>>>> controller driver support
>>>>> config: mips-allyesconfig
>>>>> (https://download.01.org/0day-ci/archive/20230308/202303082037.QPfBP64A-lkp@intel.com/config)
>>>>> compiler: mips-linux-gcc (GCC) 12.1.0
>>>>> reproduce (this is a W=1 build):
>>>>>           wget
>>>>> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
>>>>> -O ~/bin/make.cross
>>>>>           chmod +x ~/bin/make.cross
>>>>>           #
>>>>> https://github.com/intel-lab-lkp/linux/commit/391d6fc63ac65f5456e4755c9dd85232a6296285
>>>>>           git remote add linux-review
>>>>> https://github.com/intel-lab-lkp/linux
>>>>>           git fetch --no-tags linux-review
>>>>> Yinbo-Zhu/clk-clk-loongson2-add-clock-controller-driver-support/20230307-195252
>>>>>           git checkout 391d6fc63ac65f5456e4755c9dd85232a6296285
>>>>>           # save the config file
>>>>>           mkdir build_dir && cp config build_dir/.config
>>>>>           COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0
>>>>> make.cross W=1 O=build_dir ARCH=mips olddefconfig
>>>>>           COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0
>>>>> make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/
>>>>>
>>>>> If you fix the issue, kindly add following tag where applicable
>>>>> | Reported-by: kernel test robot <lkp@intel.com>
>>>>> | Link:
>>>>> https://lore.kernel.org/oe-kbuild-all/202303082037.QPfBP64A-lkp@intel.com/
>>>>>
>>>>> All errors (new ones prefixed by >>):
>>>>>
>>>>>      drivers/clk/clk-loongson2.c: In function 'loongson2_calc_pll_rate':
>>>>>>> drivers/clk/clk-loongson2.c:79:15: error: implicit declaration of
>>>>>>> function 'readq'; did you mean 'readl'?
>>>>>>> [-Werror=implicit-function-declaration]
>>>>>         79 |         val = readq(loongson2_pll_base + offset);
>>>>>            |               ^~~~~
>>>>>            |               readl
>>>>>      cc1: some warnings being treated as errors
>>>> The CONFIG_64BIT not enabled in your config file, I will add a depend
>>>> on "CONFIG_64BIT" in my clock driver to fix this compile error.
>>> My clock is for LoongArch platform, The LOONGARCH had select
>>> "CONFIG_64BIT", I will add a depend on "LOONGARCH" in my clock driver to
>>> fix this compile error.
>> No. Fix your code instead.
> 
> but the readq that ask CONFIG_64BIT is enabled,  definition in 
> include/asm-generic/io.h
> 
> so need enable CONFIG_64BIT for my clk driver.

Ah, you are right, the error was not about the cast but missing readq.
Yeah, you should depend on 64BIT. Not on Loongarch because the code
should compile test on other archs.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support
  2023-03-09  2:58   ` zhuyinbo
  2023-03-09  3:18     ` zhuyinbo
@ 2023-03-09  6:09     ` Krzysztof Kozlowski
  2023-03-09 23:47     ` Stephen Boyd
  2 siblings, 0 replies; 13+ messages in thread
From: Krzysztof Kozlowski @ 2023-03-09  6:09 UTC (permalink / raw)
  To: zhuyinbo, kernel test robot, Michael Turquette, Stephen Boyd,
	Rob Herring, linux-kernel, linux-clk, devicetree
  Cc: oe-kbuild-all, Jianmin Lv, Liu Peibao, wanghongliang, loongson-kernel

On 09/03/2023 03:58, zhuyinbo wrote:
> 
> 在 2023/3/8 下午8:16, kernel test robot 写道:
>> Hi Yinbo,
>>
>> I love your patch! Yet something to improve:
>>
>> [auto build test ERROR on clk/clk-next]
>> [also build test ERROR on robh/for-next linus/master v6.3-rc1 next-20230308]
>> [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#_base_tree_information]
>>
>> url:    https://github.com/intel-lab-lkp/linux/commits/Yinbo-Zhu/clk-clk-loongson2-add-clock-controller-driver-support/20230307-195252
>> base:   https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
>> patch link:    https://lore.kernel.org/r/20230307115022.12846-2-zhuyinbo%40loongson.cn
>> patch subject: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support
>> config: mips-allyesconfig (https://download.01.org/0day-ci/archive/20230308/202303082037.QPfBP64A-lkp@intel.com/config)
>> compiler: mips-linux-gcc (GCC) 12.1.0
>> reproduce (this is a W=1 build):
>>          wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>>          chmod +x ~/bin/make.cross
>>          # https://github.com/intel-lab-lkp/linux/commit/391d6fc63ac65f5456e4755c9dd85232a6296285
>>          git remote add linux-review https://github.com/intel-lab-lkp/linux
>>          git fetch --no-tags linux-review Yinbo-Zhu/clk-clk-loongson2-add-clock-controller-driver-support/20230307-195252
>>          git checkout 391d6fc63ac65f5456e4755c9dd85232a6296285
>>          # save the config file
>>          mkdir build_dir && cp config build_dir/.config
>>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=mips olddefconfig
>>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/
>>
>> If you fix the issue, kindly add following tag where applicable
>> | Reported-by: kernel test robot <lkp@intel.com>
>> | Link: https://lore.kernel.org/oe-kbuild-all/202303082037.QPfBP64A-lkp@intel.com/
>>
>> All errors (new ones prefixed by >>):
>>
>>     drivers/clk/clk-loongson2.c: In function 'loongson2_calc_pll_rate':
>>>> drivers/clk/clk-loongson2.c:79:15: error: implicit declaration of function 'readq'; did you mean 'readl'? [-Werror=implicit-function-declaration]
>>        79 |         val = readq(loongson2_pll_base + offset);
>>           |               ^~~~~
>>           |               readl
>>     cc1: some warnings being treated as errors
> 
> The CONFIG_64BIT not enabled in your config file, I will add a depend on 
> "CONFIG_64BIT" in my clock driver to fix this compile error.

No, that's not how it should be fixed. Fix your code instead.



Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support
  2023-03-09  2:58   ` zhuyinbo
  2023-03-09  3:18     ` zhuyinbo
  2023-03-09  6:09     ` Krzysztof Kozlowski
@ 2023-03-09 23:47     ` Stephen Boyd
  2023-03-10  8:42       ` Krzysztof Kozlowski
  2 siblings, 1 reply; 13+ messages in thread
From: Stephen Boyd @ 2023-03-09 23:47 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Michael Turquette, Rob Herring, devicetree,
	kernel test robot, linux-clk, linux-kernel, zhuyinbo
  Cc: oe-kbuild-all, Jianmin Lv, Liu Peibao, wanghongliang, loongson-kernel

Quoting zhuyinbo (2023-03-08 18:58:02)
> 
> 在 2023/3/8 下午8:16, kernel test robot 写道:
> > Hi Yinbo,
> >
[...]
> >
> >     drivers/clk/clk-loongson2.c: In function 'loongson2_calc_pll_rate':
> >>> drivers/clk/clk-loongson2.c:79:15: error: implicit declaration of function 'readq'; did you mean 'readl'? [-Werror=implicit-function-declaration]
> >        79 |         val = readq(loongson2_pll_base + offset);
> >           |               ^~~~~
> >           |               readl
> >     cc1: some warnings being treated as errors
> 
> The CONFIG_64BIT not enabled in your config file, I will add a depend on 
> "CONFIG_64BIT" in my clock driver to fix this compile error.

Do you need to use readq() here? Can you read two 32-bit registers with
readl() and put them together for a 64-bit number?

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support
  2023-03-09 23:47     ` Stephen Boyd
@ 2023-03-10  8:42       ` Krzysztof Kozlowski
  2023-03-13 18:20         ` Stephen Boyd
  0 siblings, 1 reply; 13+ messages in thread
From: Krzysztof Kozlowski @ 2023-03-10  8:42 UTC (permalink / raw)
  To: Stephen Boyd, Michael Turquette, Rob Herring, devicetree,
	kernel test robot, linux-clk, linux-kernel, zhuyinbo
  Cc: oe-kbuild-all, Jianmin Lv, Liu Peibao, wanghongliang, loongson-kernel

On 10/03/2023 00:47, Stephen Boyd wrote:
> Quoting zhuyinbo (2023-03-08 18:58:02)
>>
>> 在 2023/3/8 下午8:16, kernel test robot 写道:
>>> Hi Yinbo,
>>>
> [...]
>>>
>>>     drivers/clk/clk-loongson2.c: In function 'loongson2_calc_pll_rate':
>>>>> drivers/clk/clk-loongson2.c:79:15: error: implicit declaration of function 'readq'; did you mean 'readl'? [-Werror=implicit-function-declaration]
>>>        79 |         val = readq(loongson2_pll_base + offset);
>>>           |               ^~~~~
>>>           |               readl
>>>     cc1: some warnings being treated as errors
>>
>> The CONFIG_64BIT not enabled in your config file, I will add a depend on 
>> "CONFIG_64BIT" in my clock driver to fix this compile error.
> 
> Do you need to use readq() here? Can you read two 32-bit registers with
> readl() and put them together for a 64-bit number?

If the platform supports 64-bit reads and these are actually one
register, then readq makes sense - code is more readable, smaller, more
efficient.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support
  2023-03-10  8:42       ` Krzysztof Kozlowski
@ 2023-03-13 18:20         ` Stephen Boyd
  2023-03-14  1:07           ` zhuyinbo
  2023-03-14  6:49           ` Krzysztof Kozlowski
  0 siblings, 2 replies; 13+ messages in thread
From: Stephen Boyd @ 2023-03-13 18:20 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Michael Turquette, Rob Herring, devicetree,
	kernel test robot, linux-clk, linux-kernel, zhuyinbo
  Cc: oe-kbuild-all, Jianmin Lv, Liu Peibao, wanghongliang, loongson-kernel

Quoting Krzysztof Kozlowski (2023-03-10 00:42:47)
> On 10/03/2023 00:47, Stephen Boyd wrote:
> > Quoting zhuyinbo (2023-03-08 18:58:02)
> >>
> >> 在 2023/3/8 下午8:16, kernel test robot 写道:
> >>> Hi Yinbo,
> >>>
> > [...]
> >>>
> >>>     drivers/clk/clk-loongson2.c: In function 'loongson2_calc_pll_rate':
> >>>>> drivers/clk/clk-loongson2.c:79:15: error: implicit declaration of function 'readq'; did you mean 'readl'? [-Werror=implicit-function-declaration]
> >>>        79 |         val = readq(loongson2_pll_base + offset);
> >>>           |               ^~~~~
> >>>           |               readl
> >>>     cc1: some warnings being treated as errors
> >>
> >> The CONFIG_64BIT not enabled in your config file, I will add a depend on 
> >> "CONFIG_64BIT" in my clock driver to fix this compile error.
> > 
> > Do you need to use readq() here? Can you read two 32-bit registers with
> > readl() and put them together for a 64-bit number?
> 
> If the platform supports 64-bit reads and these are actually one
> register, then readq makes sense - code is more readable, smaller, more
> efficient.
> 

Please read the section in Documentation/driver-api/device-io.rst about
hi_lo_readq() and <linux/io-64-nonatomic-lo-hi.h>. We shouldn't need to
restrict the driver to CONFIG_64BIT. Instead, include one of these
header files to get the IO access primitives.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support
  2023-03-13 18:20         ` Stephen Boyd
@ 2023-03-14  1:07           ` zhuyinbo
  2023-03-14  6:49           ` Krzysztof Kozlowski
  1 sibling, 0 replies; 13+ messages in thread
From: zhuyinbo @ 2023-03-14  1:07 UTC (permalink / raw)
  To: Stephen Boyd, Krzysztof Kozlowski, Michael Turquette,
	Rob Herring, devicetree, kernel test robot, linux-clk,
	linux-kernel
  Cc: oe-kbuild-all, Jianmin Lv, Liu Peibao, wanghongliang,
	loongson-kernel, zhuyinbo


在 2023/3/14 上午2:20, Stephen Boyd 写道:
> Quoting Krzysztof Kozlowski (2023-03-10 00:42:47)
>> On 10/03/2023 00:47, Stephen Boyd wrote:
>>> Quoting zhuyinbo (2023-03-08 18:58:02)
>>>> 在 2023/3/8 下午8:16, kernel test robot 写道:
>>>>> Hi Yinbo,
>>>>>
>>> [...]
>>>>>      drivers/clk/clk-loongson2.c: In function 'loongson2_calc_pll_rate':
>>>>>>> drivers/clk/clk-loongson2.c:79:15: error: implicit declaration of function 'readq'; did you mean 'readl'? [-Werror=implicit-function-declaration]
>>>>>         79 |         val = readq(loongson2_pll_base + offset);
>>>>>            |               ^~~~~
>>>>>            |               readl
>>>>>      cc1: some warnings being treated as errors
>>>> The CONFIG_64BIT not enabled in your config file, I will add a depend on
>>>> "CONFIG_64BIT" in my clock driver to fix this compile error.
>>> Do you need to use readq() here? Can you read two 32-bit registers with
>>> readl() and put them together for a 64-bit number?
>> If the platform supports 64-bit reads and these are actually one
>> register, then readq makes sense - code is more readable, smaller, more
>> efficient.
>>
> Please read the section in Documentation/driver-api/device-io.rst about
> hi_lo_readq() and <linux/io-64-nonatomic-lo-hi.h>. We shouldn't need to
> restrict the driver to CONFIG_64BIT. Instead, include one of these
> header files to get the IO access primitives.

okay, I got it.

Thanks your advice!


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support
  2023-03-13 18:20         ` Stephen Boyd
  2023-03-14  1:07           ` zhuyinbo
@ 2023-03-14  6:49           ` Krzysztof Kozlowski
  2023-03-15  0:26             ` Stephen Boyd
  1 sibling, 1 reply; 13+ messages in thread
From: Krzysztof Kozlowski @ 2023-03-14  6:49 UTC (permalink / raw)
  To: Stephen Boyd, Michael Turquette, Rob Herring, devicetree,
	kernel test robot, linux-clk, linux-kernel, zhuyinbo
  Cc: oe-kbuild-all, Jianmin Lv, Liu Peibao, wanghongliang, loongson-kernel

On 13/03/2023 19:20, Stephen Boyd wrote:
>>>> The CONFIG_64BIT not enabled in your config file, I will add a depend on 
>>>> "CONFIG_64BIT" in my clock driver to fix this compile error.
>>>
>>> Do you need to use readq() here? Can you read two 32-bit registers with
>>> readl() and put them together for a 64-bit number?
>>
>> If the platform supports 64-bit reads and these are actually one
>> register, then readq makes sense - code is more readable, smaller, more
>> efficient.
>>
> 
> Please read the section in Documentation/driver-api/device-io.rst about
> hi_lo_readq() and <linux/io-64-nonatomic-lo-hi.h>. We shouldn't need to
> restrict the driver to CONFIG_64BIT. Instead, include one of these
> header files to get the IO access primitives.

These primitives are for 32bit access. Quoting: "on 32-bit
architectures". What's the point of them if the code *will never* run on
32-bit? It will be a fake choice of linux/io-64-nonatomic-lo-hi.h or
linux/io-64-nonatomic-hi-lo.h misleading users to think this was tested
on 32-bit.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support
  2023-03-14  6:49           ` Krzysztof Kozlowski
@ 2023-03-15  0:26             ` Stephen Boyd
  0 siblings, 0 replies; 13+ messages in thread
From: Stephen Boyd @ 2023-03-15  0:26 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Michael Turquette, Rob Herring, devicetree,
	kernel test robot, linux-clk, linux-kernel, zhuyinbo
  Cc: oe-kbuild-all, Jianmin Lv, Liu Peibao, wanghongliang, loongson-kernel

Quoting Krzysztof Kozlowski (2023-03-13 23:49:40)
> On 13/03/2023 19:20, Stephen Boyd wrote:
> >>>> The CONFIG_64BIT not enabled in your config file, I will add a depend on 
> >>>> "CONFIG_64BIT" in my clock driver to fix this compile error.
> >>>
> >>> Do you need to use readq() here? Can you read two 32-bit registers with
> >>> readl() and put them together for a 64-bit number?
> >>
> >> If the platform supports 64-bit reads and these are actually one
> >> register, then readq makes sense - code is more readable, smaller, more
> >> efficient.
> >>
> > 
> > Please read the section in Documentation/driver-api/device-io.rst about
> > hi_lo_readq() and <linux/io-64-nonatomic-lo-hi.h>. We shouldn't need to
> > restrict the driver to CONFIG_64BIT. Instead, include one of these
> > header files to get the IO access primitives.
> 
> These primitives are for 32bit access. Quoting: "on 32-bit
> architectures". What's the point of them if the code *will never* run on
> 32-bit?

They're there to make drivers portable.

> It will be a fake choice of linux/io-64-nonatomic-lo-hi.h or
> linux/io-64-nonatomic-hi-lo.h misleading users to think this was tested
> on 32-bit.
> 

I don't think anyone is really going to care that it hasn't been tested.
It's not like the Linux kernel driver is the source of truth for
integrating IP blocks into different architectures. If it's wrong
someone will fix it when they try to use the hardware on 32-bit systems.

Can the register handle being read/written with two 32-bit accesses? I
still don't think we've had any answer to that question. If so, pick the
one that makes the most sense and move on.

In Linux, we try to write portable drivers. This way anyone can compile
the driver on any host architecture with whatever compiler they're
using. Otherwise, they have to download a cross compiler for the target
architecture to simply build test the code. Also, the Linux kernel is
fairly portable. We try to limit architecture specific code to arch/ and
so anything in drivers/ is ideally portable code.

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2023-03-15  0:26 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20230307115022.12846-2-zhuyinbo@loongson.cn>
2023-03-08 12:16 ` [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support kernel test robot
2023-03-09  2:58   ` zhuyinbo
2023-03-09  3:18     ` zhuyinbo
2023-03-09  6:09       ` Krzysztof Kozlowski
2023-03-09  6:27         ` zhuyinbo
2023-03-09  6:37           ` Krzysztof Kozlowski
2023-03-09  6:09     ` Krzysztof Kozlowski
2023-03-09 23:47     ` Stephen Boyd
2023-03-10  8:42       ` Krzysztof Kozlowski
2023-03-13 18:20         ` Stephen Boyd
2023-03-14  1:07           ` zhuyinbo
2023-03-14  6:49           ` Krzysztof Kozlowski
2023-03-15  0:26             ` Stephen Boyd

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).