回覆: [PATCH 5/8] i3c: mipi-i3c-hci: Add support for the AST2700 I3C controller

[Billy Tsai]

> >  drivers/i3c/master/mipi-i3c-hci/vendor_aspeed.c | 234 ++++++++++++++++++++++++
> >  drivers/i3c/master/mipi-i3c-hci/vendor_aspeed.h | 174 ++++++++++++++++++

> needn't "vendor"

Okay, I will rename vendor_aspeed.[ch] to aspeed.[ch]

> > +struct aspeed_i3c_vendor_data {
> > +     void __iomem *inhouse_regs;
> > +     void __iomem *phy_regs;
> > +     struct reset_control *rst;
> > +     struct reset_control *dma_rst;
> > +     struct clk *clk;
> > +};

> should be in aspeed.c

Okay, I will move it into aspeed.c. The only place outside that file that
touches its fields directly today is the reset/clock acquisition in
probe() -- that's moving into aspeed.c as well (see below), so nothing
outside aspeed.c will need the struct layout any more.

> > +static inline bool is_aspeed(struct i3c_hci *hci)
> > +{
> > +     return of_device_is_compatible(hci->master.dev.of_node,
> > +                                     "aspeed,ast2700-i3c-hci");
> > +}

> Don't suggest is_aspeed(), you split each feature and use drvdata like
> previous QUIRK.
>
> It will become complex if new chip appear, such aspeed,ast2800-i3c-hci,

Okay, I will drop is_aspeed() and split its 9 call sites into quirk
bits set from the match table:

- HCI_QUIRK_VENDOR_BUS_SETUP: bus_init() and do_reset_and_restore()
performing vendor-specific controller setup, including clock-stall enable.

- HCI_QUIRK_BOARD_PHY_TIMINGS: bus_init() and do_reset_and_restore()
applying PHY timing settings from the board's DT properties through the
vendor-specific PHY interface. The match-table bit identifies controllers
that need this configuration path; the timing values come from DT.

- HCI_QUIRK_VENDOR_INTR_SUMMARY: reset_and_init() enabling the vendor
interrupt summary path, and probe() selecting the vendor IRQ handler
with IRQF_SHARED dropped. These have to move together: enabling the
summary path without the handler leaves vendor interrupts
unserviced.

- HCI_QUIRK_IBI_PAYLOAD_THLD: request_ibi() growing the payload IBI
termination threshold.

- HCI_QUIRK_VENDOR_RESOURCES: probe() allocating the vendor_data struct
and acquiring the reset lines/clock -- this moves into aspeed.c
wholesale, so core.c's probe() just calls one function gated by this
bit instead of touching the struct's fields.

- HCI_QUIRK_VENDOR_DAA_INDEX: the ENTDAA loop writing the DAT slot
index into the vendor DAA index register.

- HCI_QUIRK_VENDOR_TX_AVAIL: the PIO TX path using the vendor FIFO
pointer registers instead of the generic STAT_TX_THLD bit.

Billy