| Message ID | cover.1680160851.git.jk@codeconstruct.com.au (mailing list archive) |
|---|---|
| Headers | show |
| Series | i3c dw,ast2600: Add a driver for the AST2600 i3c controller | expand |
Hi Jeremy, Thanks for the change, especially IBI features in other threads. From my understanding, ASPEED AST2600 is a SoC which uses Synopsys' I3C IP core, and several different registers, especially the pull-up resistor. If so, I am wondering if this is the right place to add ast2600-i3c-master.c, given that all current three xxx-i3c-master.c drivers in this directory are from IP providers directly. What about moving it under ~/driver/soc/? Thanks Zenghu Chen On Thu, Mar 30, 2023 at 3:22 AM Jeremy Kerr <jk@codeconstruct.com.au> wrote: > > This series adds a new i3c controller driver, for the ASPEED AST2600 13c > SoC peripheral. This device is very similar to the dw i3c controller, so > we implement this by adding a little platform abstraction to the dw > driver, and then a platform implementation for ast2600. > > v2: This is a rework from an earlier series that implemented this as > part of the dw driver; I have adopted Ben Dooks' suggestion to split > into a new driver + exported hooks from the dw base. > > As always: comments, queries etc. are most welcome. > > Cheers, > > > Jeremy > > Jeremy Kerr (3): > i3c: dw: Add infrastructure for platform-specific implementations > dt-bindings: i3c: Add AST2600 i3c controller > i3c: ast2600: Add AST2600 platform-specific driver > > .../bindings/i3c/aspeed,ast2600-i3c.yaml | 72 ++++++++ > MAINTAINERS | 6 + > drivers/i3c/master/Kconfig | 14 ++ > drivers/i3c/master/Makefile | 1 + > drivers/i3c/master/ast2600-i3c-master.c | 169 ++++++++++++++++++ > drivers/i3c/master/dw-i3c-master.c | 76 ++++---- > drivers/i3c/master/dw-i3c-master.h | 55 ++++++ > 7 files changed, 360 insertions(+), 33 deletions(-) > create mode 100644 Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c.yaml > create mode 100644 drivers/i3c/master/ast2600-i3c-master.c > create mode 100644 drivers/i3c/master/dw-i3c-master.h > > -- > 2.39.2 >
Hi Zenghu, > Thanks for the change, especially IBI features in other threads. > From my understanding, ASPEED AST2600 is a SoC which uses Synopsys' > I3C IP core, and several different registers, especially the pull-up > resistor. > If so, I am wondering if this is the right place to add > ast2600-i3c-master.c, given that all current three xxx-i3c-master.c > drivers in this directory are from IP providers directly. > What about moving it under ~/driver/soc/? It's my understanding that drivers/soc/ is for smaller parts of SoC-specific functions, rather than implementing SoC driver instances for existing subsystems. I'd prefer to keep it with the i3c controller drivers; this means we can keep the dw platform API as contained as possible (ie., within drivers/i3c/master/). I expect that we will need some coordination of changes until we have the platform-specific behaviour mostly described (see the IBI series for another hook), and so coordinating changes between drivers/soc/ and drivers/i3c/ may make things more complicated than necessary. There's certainly precedence for this pattern: * the aspeed ethernet mac is provided by the ftgmac100 driver plus some SoC-specific behaviour; that's entirely within drivers/net/ * the aspeed VUART device is essentially a 16550 plus extra registers; that's entirely within drivers/tty/ Cheers, Jeremy
On 31/03/2023 10:33:35+0800, Jeremy Kerr wrote: > Hi Zenghu, > > > Thanks for the change, especially IBI features in other threads. > > From my understanding, ASPEED AST2600 is a SoC which uses Synopsys' > > I3C IP core, and several different registers, especially the pull-up > > resistor. > > If so, I am wondering if this is the right place to add > > ast2600-i3c-master.c, given that all current three xxx-i3c-master.c > > drivers in this directory are from IP providers directly. > > What about moving it under ~/driver/soc/? > > It's my understanding that drivers/soc/ is for smaller parts of > SoC-specific functions, rather than implementing SoC driver instances > for existing subsystems. > > I'd prefer to keep it with the i3c controller drivers; this means we can > keep the dw platform API as contained as possible (ie., within > drivers/i3c/master/). I expect that we will need some coordination of > changes until we have the platform-specific behaviour mostly described > (see the IBI series for another hook), and so coordinating changes > between drivers/soc/ and drivers/i3c/ may make things more complicated > than necessary. > > There's certainly precedence for this pattern: > > * the aspeed ethernet mac is provided by the ftgmac100 driver plus some > SoC-specific behaviour; that's entirely within drivers/net/ > > * the aspeed VUART device is essentially a 16550 plus extra registers; > that's entirely within drivers/tty/ > I confirm this has to be in drivers/i3c