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

[Youngmin Nam <ym0914@gmail.com>]

When I think about this patch, I was motivated by below patch.

commit 135f07c3252dc77d0245714d0b413ecc711cd823
Author: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
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@samsung.com>
    Reviewed-by: Tomasz Figa <t.figa@samsung.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 

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. 

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
> 
>