Re: [PATCH] spi: s3c64xx: Get fifosize via device tree

[Krzysztof Kozlowski <k.kozlowski@xxxxxxxxxxx>]

2016-02-14 22:22 GMT+09:00 Youngmin Nam <ym0914@xxxxxxxxx>:
> When I think about this patch, I was motivated by below patch.
>
> commit 135f07c3252dc77d0245714d0b413ecc711cd823
> Author: Naveen Krishna Chatradhi <ch.naveen@xxxxxxxxxxx>
> Date:   Mon Jul 14 17:07:16 2014 +0530
>
>     serial: samsung: get fifosize via device tree
>
>     UART modules on some SoCs only differ in the fifosize of each
>     UART channel. Its useless to duplicate the drv_data structure
>     or create a compatible name for such a change.
>
>     We can get fifosize via the device tree nodes (not mandating it).
>
>     Also updates the documentation.
>
>     Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@xxxxxxxxxxx>
>     Reviewed-by: Tomasz Figa <t.figa@xxxxxxxxxxx>
>     Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
>
> I thought we can apply above patch to SPI also.
> The fifosize should be required for fifo_lvl_mask value in SPI of Exynos series.
> I think this patch is useful when we port for the new exynos7xxx SoC come out.
> Because SPI of new exynos7xxx SoC has only difference on fifosize.
>

Please, do not top post.

You skipped some of arguments from my reply instead saying "patch is
useful". That is not a sufficient argument for my doubts... but let me
state again the most important question:

What happens if this *optional* property is not present on board with
Exynos7? What value should be used?

Best regards,
Krzysztof

> Thanks.
>
>
> On 2016년 02월 14일 17:31, Krzysztof Kozlowski wrote:
>> W dniu 14.02.2016 o 17:13, Youngmin Nam pisze:
>>> Hello Krzysztof,
>>>
>>> As you mentioned, spi fifosize is not configurable in the given SoC.
>>> The point is we can set fifosize without changing driver code.
>>> For example, if some SoC in exynos7 series has different spi fifosize of on each channel with current
>>> our compatible, we can't cover this situation without adding new compatible into spi driver code.
>>
>> Yes, because new SoC is not compatible with old ones... so a new
>> compatible is required!
>>
>>> Whenever new SoC kind of exynos7 come out, we should add new compatible into driver code only for fifosize change.
>>> I think this is not efficient. I think we can reduces this works through DT handling.
>>
>> I agree that this is not the most efficient possible way of setting some
>> specific properties of devices but this is the way how DT works.
>>
>> What if the property is not present on board with Exynos7? What value
>> should be used?
>>
>> You did not want to add a new compatible but you are adding a
>> compatible-like property which apparently is required for some devices.
>>
>> That looks bad. It's easy to make a mistake, messes with compatibles.
>>
>> Best regards,
>> Krzysztof
>>
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html