Le 23/08/2022 à 17:14, Baoquan He a écrit : > On 08/23/22 at 05:24am, Christophe Leroy wrote: >> >> >> Le 23/08/2022 à 02:20, Baoquan He a écrit : >>> On 08/22/22 at 06:25am, Christophe Leroy wrote: >>>> >>>> >>>> Le 20/08/2022 à 02:31, Baoquan He a écrit : >>>>> In some architectures, there are ARCH specifici io address mapping >>>>> handling when calling ioremap() or ioremap_prot(), e.g, arc, ia64, >>>>> openrisc, s390, sh. >>>>> >>>>> In oder to convert them to take GENERIC_IOREMAP method, we need change >>>>> the return value of hook ioremap_allowed() and iounmap_allowed(). >>>>> Meanwhile, rename them to arch_ioremap() and arch_iounmap() to reflect >>>>> their current behaviour. >>> >>> Thanks for reviewing. >>> >>>> >>>> Please don't just say you need to change the return value. Explain why. >>> >>> The 1st paragraph and the sentence 'In oder to convert them to take >>> GENERIC_IOREMAP method' tell the reason, no? >> >> What I would like to read is _why_ you need to change the return value >> in order to convert to GENERIC_IOREMAP > > I rephrase the log as below, it's OK to you? Or please help check and > tell what I need to improve to better explain the reason. > > ==== > The current io[re|un]map_allowed() hooks are used to check if the > io[re|un]map() actions are qualified to proceed when taking > GENERIC_IOREMAP way to do ioremap()/iounmap(). Otherwise io[re|un]map() > will return NULL. > > On some architectures like arc, ia64, openris, s390, sh, there are > ARCH specific io address mapping to translate the passed in physical > address to io address when calling ioremap(). In order to convert > these architectures to take GENERIC_IOREMAP way to ioremap(), we need > change the return value of hook ioremap_allowed() and iounmap_allowed(). > With the change, we can move the architecture specific io address > mapping into ioremap_allowed() hook, and give the mapped io address > out to let ioremap_prot() return it. While at it, rename the hooks to > arch_ioremap() and arch_iounmap() to reflect their new behaviour. > ==== > That looks more in line with the type of explanation I foresee in the commit message, thanks. Christophe
