On Mon, 26 Nov 2018 12:06:24 +0000 vitor <vitor.soares@xxxxxxxxxxxx> wrote: > Hi Boris, > > > On 23/11/18 12:50, Boris Brezillon wrote: > > On Fri, 23 Nov 2018 12:39:31 +0000 > > vitor <vitor.soares@xxxxxxxxxxxx> wrote: > > > >> Hi Boris, > >> > >> > >> On 22/11/18 20:02, Boris Brezillon wrote: > >>> On Thu, 22 Nov 2018 17:54:54 +0000 > >>> Vitor Soares <vitor.soares@xxxxxxxxxxxx> wrote: > >>> > >>>> From: Vitor Soares <soares@xxxxxxxxxxxx> > >>>> > >>>> This patch slipts dw-i3c-master.c into three pieces: > >>>> dw-i3c-master.c - contains the code that interacts directly with the > >>>> core in master mode. > >>>> > >>>> dw-i3c-platdrv.c - contains the code specific to the platform driver. > >>>> > >>>> dw-i3c-core.h - contains the definitions and declarations shared by > >>>> dw-i3c-master and dw-i3c-platdrv > >>>> > >>>> This patch will allow SOC integrators to add their code specific to > >>>> DesignWare I3C IP. > >>> Isn't it too early to do this change? Can't we wait until we have a SoC > >>> that actually embeds this IP? > >> > >> I'm trying to turn it more flexible so the other can reuse the code. > > Looking at the separation you've done here, I don't see why you need > > it. All the resources you request are generic, so why not just adding a > > new compat in the of_match_table? > > I understand your point. > > > I'm just following what it's done in others Synopsys drivers and what I > expect is that in the future we will have the same for the I3C. > > Some of the current generic functions might be override according with > SoC requirements (e.g i2c-designware, pcie-designware). > > > for now what do you prefer? > I prefer that we keep the driver as is until we actually need to split things up. > >> > >>> > >>>> Signed-off-by: Vitor Soares <soares@xxxxxxxxxxxx> > >>>> --- > >>>> drivers/i3c/master/Kconfig | 9 +- > >>>> drivers/i3c/master/Makefile | 5 +- > >>>> drivers/i3c/master/dw-i3c-core.h | 214 ++++++++++++++++++++++++++ > >>>> drivers/i3c/master/dw-i3c-master.c | 299 ++---------------------------------- > >>>> drivers/i3c/master/dw-i3c-platdrv.c | 112 ++++++++++++++ > > Just realized the driver is named dw-i3c-master, while the cadence > > driver is named i3c-master-cdns.c. I'll send a patch to make that > > consistent and follow the initial naming scheme: i3c-master-<ipname>.c. > > As I shared with you in previous email, the structure that I have in > mind is this one: > > - core.h (or common.h, any though?) > > - common.c > > - master.c > > - slave.c > > > so for me doesn't make sense to have for instance: i3c-master-dw-slave.c If you have several files and they're all placed in a dw/ subdir, then I agree, prefixing everything with i3c-master- is useless, as you'll have to define a custom rule to create the i3c-master-dw.ko object. When there's a single source file, and this source file is directly used to create a .ko, we need this prefix, otherwise we would have dw.ko, and this would basically conflict with any other designware driver that does not have a proper prefix. > > But seeing what is already in the kernel I wasn't coherent and it should > be named to i3c-designware-master.c Actually it's i3c-master-designware.c (or i3c-master-dw.c) if we follow what's been done for the cadence driver. > > > or > > > follow this https://lkml.org/lkml/2017/7/12/430 And I agree with Linus on this, except that does not apply to single source file drivers. > > > This topic rise another one related with the master folder. I understand > that now the subsystem doesn't have slave support but the name is > limited. Isn't better to have something like controller or busses? What > do you have in mind for the slave? drivers/i3c/slave/... for slave drivers and drivers/i3c/slave.c for the framework, just like we have drivers/i3c/master/ for master controller drivers and drivers/i3c/master.c.
