On Mon, Nov 14, 2022 at 07:14:48PM +0300, Dmitry Baryshkov wrote: > On 14/11/2022 18:38, Johan Hovold wrote: > > On Mon, Nov 14, 2022 at 06:19:25PM +0300, Dmitry Baryshkov wrote: > >> On 14/11/2022 17:18, Johan Hovold wrote: > >>> On Mon, Nov 14, 2022 at 03:07:41PM +0100, Krzysztof Kozlowski wrote: > >>>> On 14/11/2022 14:27, Johan Hovold wrote: > >>>>> On Fri, Nov 11, 2022 at 04:17:29PM +0100, Krzysztof Kozlowski wrote: > >>>>>> On 11/11/2022 10:24, Johan Hovold wrote: > > > >>>>> I noticed that several bindings leave the clock indexes unspecified, or > >>>>> have header files defining some or all of them. I first added a QMP > >>>>> header but that seemed like overkill, especially if we'd end up with > >>>>> one header per SoC (cf. the GCC headers) due to (known and potential) > >>>>> platform differences. > > > >>>>> Shall I add back a shared header for all PHYs handled by this driver > >>>>> (another implementation detail) even if this could eventually lead to > >>>>> describing clocks not supported by a particular SoC (so such constraints > >>>>> would still need to be described by the binding somehow): > >>>>> > >>>>> /* QMP clocks */ > >>>>> #define QMP_USB3_PIPE_CLK 0 > >>>>> #define QMP_DP_LINK_CLK 1 > >>>>> #define QMP_DP_VCO_DIV_CLK 2 > >> > >> Maybe QMP_COMBO_USB3_PIPE_CLK, QMP_COMBO_DP_LINK_CLK, > >> QMP_COMBO_DP_VCO_DIV_CLK? > > > > "COMBO" is just the name of the Linux driver and does not belong in the > > binding. > > We do not have any standard (iow, coming from the docs) name, so we can > invent it on our own. I still think the naming should reflect the hardware and not the Linux implementation if this is going into the binding. And the USB4_USB3_DP defines are going to be a superset of USB3_DP (as far we know know). > >> I'll then extend this header with QMP_UFS_RX_SYMBOL_0_CLK > >> QMP_UFS_RX_SYMBOL_1_CLK and QMP_UFS_TX_SYMBOL_0_CLK. > > > > Yeah, I had those in mind when creating the header and using a generic > > QMP prefix (even if I didn't end up using the header in v1). > > > > This could just be mapping of (arbitrary) QMP indexes to clocks and we > > use it for USB3, DP, UFS and later also USB4. > > > > This will however mean that the indexes are not necessarily zero-based > > and consecutive for a specific SoC and PHY. But that's perhaps a > > non-issue (cf. the PHY_TYPE defines). > > Ugh. Please, no. We have symbol clocks for UFS PHY, USB+DP clocks for > USB+DP PHY, but let's not go for the unified clocks index definition. Yeah, this is the kind of issues I wanted to avoid by not using a per SoC header for three clocks which will almost always use the same indexes. Because how can you be sure that your unified per-PHY type defines will never have to be amended? Or some index left out? The only way then is to have per-SoC defines which is a pain to maintain (just consider that driver mapping table when some odd SoC shows up). Johan
