Hi Ben,

Thanks for taking a look at the patch. My responses inline (just
re-ordered, simple stuff first)

quoted

  struct dw_i3c_i2c_dev_data {

@@ -612,6 +623,12 @@ static int dw_i3c_master_bus_init(struct i3c_master_controller *m)

        u32 thld_ctrl;
        int ret;
  
+       if (master->platform_ops && master->platform_ops->init) {
+               ret = master->platform_ops->init(master);
+               if (ret)
+                       return ret;
+       }

I'd rather have a "default" set of ops than have all this checking for
NULL pointers all over the place.

Yep, that's a better structure, changed for v2.

quoted

@@ -1181,6 +1205,18 @@ static int dw_i3c_probe(struct platform_device *pdev)

        master->maxdevs = ret >> 16;
        master->free_pos = GENMASK(master->maxdevs - 1, 0);
  
+       /* match any platform-specific ops */
+       match = of_match_node(dw_i3c_master_of_match, pdev->dev.of_node);
+       if (match && match->data)
+               master->platform_ops = match->data;

I'm sure there's a of_device_get_match_data() which would have
both removed hte need to move the match table around and the
call to of_match_node().

That's the one I was looking for! Thanks for the pointer, I have updated
in v2.

quoted

@@ -241,6 +241,17 @@ struct dw_i3c_master {

        char version[5];
        char type[5];
        u8 addrs[MAX_DEVS];
+
+       /* platform-specific data */
+       const struct dw_i3c_platform_ops *platform_ops;
+       union {
+       } pdata;
+
+};
+
+struct dw_i3c_platform_ops {
+       int (*probe)(struct dw_i3c_master *i3c, struct platform_device *pdev);
+       int (*init)(struct dw_i3c_master *i3c);
  };

Given the comment below having this and the main probe defined in a 
header so users can just call in and we don't have to change the
main code here every time someone comes up with their own
special way of handing this?

I'm not sure I 100% understand the intention here - is it that we'd
split the platform-specific code into entirely new drivers, and have
those call into dw_i3c_probe() (presumably doing a bit of custom init
either before or after that call)?

If so: I think the platform support should stay fairly minimal, so I'm
not sure that warrants a new driver for each instance. In the ast2600
case it's just a couple of extra reg writes in the i3c init path. I'd be
reluctant to split that out completely at this stage - but if this does
grow, we can certainly reconsider.

Also, I'd like to allow for the case where the platform-specific parts
may access the fields of struct dw_i3c_master; with this approach we
don't need to expose that struct outside of the single driver.

Cheers,


Jeremy