Chen Wang

> @unicornx 可以帮忙整理出一定的commit，PR的规范吗？如能形成issue，可以钉在置顶上。谢谢 好的，我找时间来整理一下

现在有maintainer制度了，希望每个maintainer 能把把关，close first。

以后 PR 上 reviewer 提出的问题，建议 PR 的 author 都必须给予答复，accept OR reject，没有答复就开始重新提交是一种不礼貌的行为。

你测试过或者考虑过 arm 吗？ ARM 大核默认的配置并没有开启 `RT_USING_SMART`，所以原来的逻辑是针对 ARM 大核，启用 ioremap，对 RISC-V 不启用 ioremap。而且 ARM 大核启用 ioremap 无关是否是 smart。 你现在的修改后的行为是：只有 smart 才会启用 ioremap 这看上去对原有的逻辑是有影响的。 我觉得还不如直接定义一个配置快关用于决定是否启用 ioremap，因为目前来看 ioremap 是一个独立的功能开关，和是否 smart 并无直接关系。

目前的代码看上去有点套娃的感觉。 `bsp/cvitek/drivers/drv_ioremap.h` 里 `DRV_IOREMAP` 用 `RT_USING_SMART` 进行了控制 ```c #ifdef RT_USING_SMART #include #define DRV_IOREMAP(addr, size) rt_ioremap(addr, size) #define DRV_IOUNMAP(addr) rt_iounmap(addr) #else #define DRV_IOREMAP(addr, size) (addr) #define DRV_IOUNMAP(addr) #endif ``` `bsp/cvitek/drivers/drv_pinmux.c` 里同样用相同的宏...

`PINMUX_BASE` 会被很多地方调用，如果在启用 ioremap 下每次都要判断 `pinmux_base` 感觉有点浪费。 在启用 ioremap 的情况下，是否可以找个地方对 `pinmux_base` 进行初始化一次，然后 `PINMUX_BASE` 就直接定义为 `pinmux_base`，也就是说： 下面这段代码找个 board 初始化的地方尽早初始化，在 pinmux_config 被使用之前，或者在 pinmux_config 中只确保调用一次 ```c static rt_ubase_t pinmux_base = RT_NULL; #ifdef BSP_USING_IOREMAP #include...

> 目前的代码看上去有点套娃的感觉。 > > `bsp/cvitek/drivers/drv_ioremap.h` 里 `DRV_IOREMAP` 用 `RT_USING_SMART` 进行了控制 > > ```c > #ifdef RT_USING_SMART > #include > > #define DRV_IOREMAP(addr, size) rt_ioremap(addr, size) > #define DRV_IOUNMAP(addr) rt_iounmap(addr) >...

Also， 你的 commit 信息一如既往地简单，只有一句话，该怎么写我建议你读一下 https://gist.github.com/finalfantasia/bd0070673ca27e5f7473。 不用怕麻烦，这种工作只要做好一次，养成习惯，后面就很简单了。 后面如果仍然要邀请我 review 的话，我首先就看 commit 信息，写得不清楚我就 **不** approve 了。：）

我在第一次 review 中提出的 [套娃 问题](https://github.com/RT-Thread/rt-thread/pull/9244#issuecomment-2249182799) 没有答复，也没有解决。

commit 文字中的 “添加理由” 和 “使用建议” 都是属于解决方案的内容，但是作为一个 new feature 开发，设计上缺少需求描述 建议对于 new feature 开发，commit 文字中至少含有以下几部分： - 为什么要做这个 PR - 你的修改思路是什么 - 其他注意事项，譬如测试上是否有什么需要注意等 另外作为完整性，请在 commit 中注明，对于 c906L, 因为是 RTOS，不考虑 ioremap。