On 26/05/2023 21:30, Conor Dooley wrote: > On Fri, May 26, 2023 at 02:24:21PM -0500, Adam Ford wrote: >> On Fri, May 26, 2023 at 1:19 PM Conor Dooley <conor@xxxxxxxxxx> wrote: >>> On Thu, May 25, 2023 at 10:05:59PM -0500, Adam Ford wrote: > >>>> description: >>>> - DSIM high speed burst mode frequency. >>>> + DSIM high speed burst mode frequency when connected to devices >>>> + that support burst mode. If absent, the driver will use the pixel >>>> + clock from the attached device or bridge. >>> >>> I'd rather this description did not say anything about drivers. >>> How about: >>> If absent, the pixel clock from the attached device or bridge >>> will be used instead. >> >> That makes sense. I can do that. >> >> "DSIM high speed burst mode frequency (optional). If absent, the pixel >> clock from the attached device or bridge will be used instead." >> >>> Or perhaps "must be used"? Ditto below. >> >> "Must be" implies to me that the user needs to set something. Are you >> ok with the proposed suggestion above? >>> >>> Description aside, the removal seems to be backwards compatible - but >>> can every device that this binding supports work using an "attached >>> device or bridge", or are these properties going to be required for >>> certain compatibles? >> >> From what I can tell, the assumption is that the DSIM driver was >> expecting it to attach to panels in the past. With the additional >> patch series, the DSIM can attach to bridge parts without a hard-coded >> set of clocks. I don't expect the existing Exynos devices to change, >> but I also don't know what would preclude those SoC's from attaching >> to a bridge should someone want to design a new product around them. > > Okay, that seems fair. With your revised wording, > Reviewed-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx> > >> >> I'll wait a couple days for more feedback and send patch V2 with just >> this patch since the rest of the series has been applied to the drm >> branch. > > Sounds good. Krzysztof will hopefully be able to take a look then too to > make sure I am not making a hames of things. We should avoid references to driver, because bindings are used also in other projects where driver can behave differently. Also "driver" is then ambiguous - which driver do you mean? Please re-phrase. Best regards, Krzysztof