Re: [RFC v3 02/13] [media] exynos5-fimc-is: Add Exynos5 FIMC-IS device tree bindings documentation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



W dniu 2013-08-08 22:34, Stephen Warren pisze:
On 08/05/2013 04:37 PM, Sylwester Nawrocki wrote:
On 08/05/2013 06:53 PM, Stephen Warren wrote:
On 08/03/2013 03:41 PM, Sylwester Nawrocki wrote:
On 08/02/2013 05:02 PM, Arun Kumar K wrote:
The patch adds the DT binding documentation for Samsung
Exynos5 SoC series imaging subsystem (FIMC-IS).

diff --git
a/Documentation/devicetree/bindings/media/exynos5-fimc-is.txt
b/Documentation/devicetree/bindings/media/exynos5-fimc-is.txt
new file mode 100644
index 0000000..49a373a
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/exynos5-fimc-is.txt
@@ -0,0 +1,52 @@
+Samsung EXYNOS5 SoC series Imaging Subsystem (FIMC-IS)
+------------------------------------------------------
+
+The camera subsystem on Samsung Exynos5 SoC has some changes relative
+to previous SoC versions. Exynos5 has almost similar MIPI-CSIS and
+FIMC-LITE IPs but has a much improved version of FIMC-IS which can
+handle sensor controls and camera post-processing operations. The
+Exynos5 FIMC-IS has a dedicated ARM Cortex A5 processor, many
+post-processing blocks (ISP, DRC, FD, ODC, DIS, 3DNR) and two
+dedicated scalers (SCC and SCP).

So there are a lot of blocks mentioned there, yet the binding doesn't
seem to describe most of it. Is the binding complete?

Thanks for the review Stephen.

No, the binding certainly isn't complete, it doesn't describe the all
available IP blocks. There are separate MMIO address regions for each
...
So while we could list all the devices, we decided not to do so.
Because it is not needed by the current software and we may miss some
details for case where the whole subsystem is controlled by the host
CPU (however such scenario is extremely unlikely AFAICT) which then
would be impossible or hard to change.

Yes, that's probably a good approach.

I guess we should list all available devices, similarly as it's done
in Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt.

And then should they just be disabled through the status property
if they are not needed in the Linux driver ? I guess it is more
sensible than marking them as optional and then not listing them
in dts at all ?

If you can define complete bindings for those nodes, it might make sense
to do that. If the devices are perhaps complex to represent and hence
you might not be able to come up with complete bindings for them right
now, it may indeed be better to simply not mention the devices you don't
care about for now.

We would prefer to start with a minimal binding, this would minimize
possible issues in future IMHO, as the subsystem is pretty complex.
Then, if detailed H/W description is required the firmware would need
to be updated, which would have been backward compatible.
We could probably come up with a complete binding, but there is a good
chance something could be done wrong, as that couldn't be actually
tested. It's not trivial to make this all work on the host CPU while
most of the detailed H/W knowledge stays on the firmware teams side.

+pmu subnode
+-----------
+
+Required properties:
+ - reg : should contain PMU physical base address and size of the
memory
+         mapped registers.

I think you need a compatible value for this. How else is the node
identified? The node name probably should not be used for identification.

Of course the node name is currently used for identification. There is no
compatible property because this pmu node is used to get hold of only part
of the Power Management Unit registers, specific to the FIMC-IS.
The PMU has more registers that also other drivers would be interested in,
e.g. clocks or USB.

I believe the correct way to solve this is for there to be a standalone
PMU node at the appropriate location in DT, and for the FIMC bindings to
reference that other node by phandle.

Right now, the FIMC driver SW can manually follow the phandle, look at
the reg property, and map that itself. Later down the road, you could
instantiate a true PMU driver, and have the FIMC driver look up that
driver, and call APIs on it. This change can be made without requiring
any changes to the DT binding. That way, you aren't introducing a fake
PMU node into the FIMC bindings just to satisfy internal Linux driver
details.

That sounds reasonable. It lets us to keep the DT binding stable and
at the same move forward with the FIMC-IS while the PMU part is being
worked on. Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux