On Thu, Jun 27, 2024 at 04:51:22PM +0100, Conor Dooley wrote: > On Thu, Jun 27, 2024 at 03:41:26AM +0300, Serge Semin wrote: > > + clocks: > > + description: > > + Both MCI and APB3 interfaces are supposed to be equipped with a clock > > + source connected via the clk_csr_i line. > > + > > + PCS/PMA layer can be clocked by an internal reference clock source > > + (phyN_core_refclk) or by an externally connected (phyN_pad_refclk) clock > > + generator. Both clocks can be supplied at a time. > > + minItems: 1 > > + maxItems: 3 > > + > > + clock-names: > > + oneOf: > > + - minItems: 1 > > + items: > > + - enum: [core, pad] > > + - const: pad > > + - minItems: 1 > > + items: > > + - const: pclk > > + - enum: [core, pad] > > + - const: pad > > While reading this, I'm kinda struggling to map "clk_csr_i" to a clock > name. Is that pclk? And why pclk if it is connected to "clk_csr_i"? Right. It's "pclk". The reason of using the "pclk" name is that it has turned to be a de-facto standard name in the DT-bindings for the peripheral bus clock sources utilized for the CSR-space IO buses. Moreover the STMMAC driver responsible for the parental DW *MAC devices handling also has the "pclk" name utilized for the clk_csr_i signal. So using the "pclk" name in the tightly coupled devices (MAC and PCS) for the same signal seemed a good idea. > If two interfaces are meant to be "equipped" with that clock, how come > it is optional? I'm probably missing something... MCI and APB3 interfaces are basically the same from the bindings pointer of view. Both of them can be utilized for the DW XPCS installed on the SoC system bus, so the device could be accessed using the simple MMIO ops. The first "clock-names" schema is meant to be applied on the DW XPCS accessible over an _MDIO_ bus, which obviously doesn't have any special CSR IO bus. In that case the DW XPCS device is supposed to be defined as a subnode of the MDIO-bus DT-node. The second "clock-names" constraint is supposed to be applied to the DW XPCS synthesized with the MCI/APB3 CSRs IO interface. The device in that case should be defined in the DT source file as a normal memory mapped device. > > Otherwise this binding looks fine to me. Shall I add a note to the clock description that the "clk_csr_i" signal is named as "pclk"? I'll need to resubmit the series anyway. Thanks -Serge(y) > > Wee bit confused, > Conor.
