Hi Rob,
Thanks for the review...
[Snip]
> > -- a/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
> >> > +++ b/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
> >> > @@ -66,6 +66,8 @@ Optional child node properties:
> >> > Optional child node properties for VDMA:
> >> > - xlnx,genlock-mode: Tells Genlock synchronization is
> >> > enabled/disabled in hardware.
> >> > +- xlnx,fstore-config: Tells Whether Frame Store Configuration is
> >> > + enabled/disabled in hardware.
> >>
> >> What's the default (when not present)? That should be the most common
> case.
> >> Looks like the code treats this as bool, but that's not clear here.
> >> The name is not clear what it is doing. Enabling or disabling the feature?
> >
> > Default value is zero...
> > When this property is present it tells hardware is configured for frame
> > store
> configuration.
>
> So most people will not want "frame store configuration"?
Since the driver is for SoftIP (I mean fpga ip) default h/w configuration of
this IP is frame store
Configuration disabled that's in the driver making default value of this
configuration as zero.
If users are trying a use case where this configuration should be enabled but
in the h/w it is disabled.
In this case driver will warn users to enable this frame store configuration in
the h/w.
So that users can enable it in their h/w.
Please let me know if the above expiation is not clear will explain in detail...
>
> > Will fix the explanation part in the next version like below.
> > xlnx,fstore-config: Tells hardware is configured for frame store
> > configuration.
> > Is the above explanation clear???
>
> No, I mean make it obvious from the name of the property:
> xlnx,fstore-config-enable or xlnx,fstore-enable
>
> And the description needs to say it is boolean.
Sure will fix in the next version...
Regards,
Kedar.
>
> Rob
