On Thu, Mar 24, 2022 at 09:39:49AM +0800, Sui Jingfeng wrote: > > On 2022/3/23 04:49, Rob Herring wrote: > > On Tue, Mar 22, 2022 at 12:29:16AM +0800, Sui Jingfeng wrote: > > > From: suijingfeng <suijingfeng@xxxxxxxxxxx> > > > > > > There is a display controller in loongson's LS2K1000 SoC and LS7A1000 > > > bridge chip, the display controller is a PCI device in those chips. It > > > has two display pipes but with only one hardware cursor. Each way has > > > a DVO interface which provide RGB888 signals, vertical & horizontal > > > synchronisations, data enable and the pixel clock. Each CRTC is able to > > > scanout from 1920x1080 resolution at 60Hz, the maxmium resolution is > > > 2048x2048 according to the hardware spec. Loongson display controllers > > > are simple which require scanout buffers to be physically contiguous. [...] > > > + val |= mask; > > > + else > > > + val &= ~mask; > > > + writeb(val, li2c->dat_reg); > > Shouldn't you set the data register low first and then change the > > direction? Otherwise, you may be driving high for a moment. However, if > > high is always done by setting the direction as input, why write the > > data register each time? I'm assuming whatever is written to the dat_reg > > is maintained regardless of pin state. > > > When the pin is input, i am not sure value written to it will be preserved. > > I'm worry about it get flushed by the external input value. > > Because the output data register is same with the input data register( > offset is 0x1650). > > The hardware designer do not provided a separation. Usually for GPIO data registers the read value is current pin state regardless of direction and the written value is what to drive as an output. But your h/w could be different. > > > + > > > + /* Optional properties which made the driver more flexible */ > > > + of_property_read_u32(i2c_np, "udelay", &udelay); > > > + of_property_read_u32(i2c_np, "timeout", &timeout); > > These aren't documented. Do you really need them in DT? > > Yes, in very rare case: > > When debugging, sometimes one way I2C works, another way I2C not on specific > board. This is not specific to you, so why do you solve it in a way that only works for you? If you want to add tuning parameters to the i2c bit algorithm, why don't you do so in a way that works for all users? I'm sure the I2C maintainer and others have some opinion on this, but they'll never see it hidden away in some display driver. > and you want to see what will happen if you change it from 5 to 2. > > modify device tree is enough, have to recompile the kernel and driver > modules every time. Modifying the DT is not the easiest way to debug either. > It is optional through. Lots of properties are optional, what's your point? > Please do not ask me to document such a easy thing, Everything must be documented. There's nothing more to discuss. > DT itself is a documention, human readable, it already speak for itself. It is machine readable too. Undocumented properties generate warnings now. Rob
