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. Thanks, Conor.
Attachment:
signature.asc
Description: PGP signature
