On 08/08/2013 03:19 PM, Tomasz Figa wrote:
> Hi Stephen,
>
> On Thursday 08 of August 2013 15:00:52 Stephen Warren wrote:
>> On 08/05/2013 06:26 AM, Marek Szyprowski wrote:
>>> MFC driver use custom bindings for managing reserved memory. Those
>>> bindings are not really specific to MFC device and no even well
>>> discussed. They can be easily replaced with generic, platform
>>> independent code for handling reserved and contiguous memory.
>>>
>>> Two additional child devices for each memory port (AXI master) are
>>> introduced to let one assign some properties to each of them. Later
>>> one
>>> can also use them to assign properties related to SYSMMU controllers,
>>> which can be used to manage the limited dma window provided by those
>>> memory ports.
>>>
>>> diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt
>>> b/Documentation/devicetree/bindings/media/s5p-mfc.txt
>>>
>>> +The MFC device is connected to system bus with two memory ports (AXI
>>> +masters) for better performance. Those memory ports are modelled as
>>> +separate child devices, so one can assign some properties to them
>>> (like +memory region for dma buffer allocation or sysmmu controller).
>>> +
>>>
>>> Required properties:
>>> - compatible : value should be either one among the following
>>>
>>> (a) "samsung,mfc-v5" for MFC v5 present in Exynos4 SoCs
>>> (b) "samsung,mfc-v6" for MFC v6 present in Exynos5 SoCs
>>>
>>> + and additionally "simple-bus" to correctly initialize child
>>> + devices for memory ports (AXI masters)
>>
>> simple-bus is wrong here; the child nodes aren't independent entities
>> that can be instantiated separately from their parent and without
>> depending on their parent.
>
> I fully agree, but I can't think of anything better. Could you suggest a
> solution that would cover our use case:
>
> The MFC IP has two separate bus masters (called here and there memports).
> On some SoCs, each master must use different memory bank to meet memory
> bandwidth requirements for higher bitrate video streams. This can be seen
> as MFC having two DMA subdevices, which have different DMA windows.
>
> On Linux, this is handled by binding two appropriate CMA memory regions to
> the memports, so the driver can do DMA allocations on behalf of particular
> memport and get appropriate memory for it.
I don't see what that has to do with simple-bus. Whatever parses the
node of the MFC can directly read from any contained property or child
node; there's no need to try and get the core DT tree parser to
enumerate the children.
If you actually need a struct platform_device for each of these child
nodes (which sounds wrong, but I'm not familiar with the code), then
simply have the driver call of_platform_populate() itself at the
appropriate time. But that's not going to work well unless the child
nodes have compatible values, which doesn't seem right given their purpose.
>>> - - samsung,mfc-r : Base address of the first memory bank used by MFC
>>> - for DMA contiguous memory allocation and its size.
>>> -
>>> - - samsung,mfc-l : Base address of the second memory bank used by
>>> MFC
>>> - for DMA contiguous memory allocation and its size.
>>
>> These properties shouldn't be removed, but simply marked deprecated. The
>> driver will need to continue to support them so that old DTs work with
>> new kernels. The binding must therefore continue to document them so
>> that the old DT content still makes sense.
>
> I tend to disagree on this. For Samsung platforms we've been trying to
> avoid DT bindings changes as much as possible, but I'd rather say that
> device tree was coupled with kernel version it came from, so Samsung-
> specific bindings haven't been fully stabilized yet, especially since we
> are still at discussion stage when it's about defining processes for
> binding staging and stabilization.
Well, that's why everyone is shouting at ARM for abusing DT...
> I would rather see this patch as part of work on Samsung DT binding
> janitoring or maybe even sanitizing in some cases, like this one, when the
> old (and IMHO bad) MFC binding was introduced without proper review. I
> don't think we want to support this kind of brokenness anymore, especially
> considering the hacks which would be required by such support (see
> original implementation of this binding and code required in board file).
>
>>> +Two child nodes must be defined for MFC device. Their names must be
>>> +following: "memport-r" and "memport-l" ("right" and "left"). Required
>>
>> Node names shouldn't have semantic meaning.
>
> What about bus-master-0 and bus-master-1?
Just "bus-master" for each might make sense. Use reg properties to
differentiate the two?
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
