[PATCH v2 1/1] dfu: remove UPDATE_TFTP

[Marek Vasut]

On 8/28/20 12:00 PM, Lukasz Majewski wrote:
> Hi Marek,

Hi,

>> On 8/28/20 11:11 AM, Heinrich Schuchardt wrote:
>>> On 28.08.20 10:42, Marek Vasut wrote:  
>>>> On 8/28/20 4:32 AM, Heinrich Schuchardt wrote:  
>>>>> On 7/21/20 8:02 PM, Heinrich Schuchardt wrote:  
>>>>>> Using UPDATE_TFTP the firmware can be updated from TFTP by
>>>>>> writing to NOR flash. The same is possible by defining a dfu
>>>>>> command in CONFIG_PREBOOT.
>>>>>>
>>>>>> The dfu command cannot only write to NOR but also to other
>>>>>> devices. In README.dfutfp UPDATE_TFTP has been marked as
>>>>>> deprecated. It is not used by any board.
>>>>>>
>>>>>> Remove TFTP update via CONFIG_UPDATE_TFTP.
>>>>>>
>>>>>> Adjust the documentation.
>>>>>>
>>>>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>>>>> ---
>>>>>> v2:
>>>>>> 	rework the documentation  
>>>>>
>>>>> On 8/28/20 12:17 AM, Marek Vasut wrote (in reply to a pull
>>>>> request):  
>>>>>> Also note that the UPDATE_TFTP is being actively used, why is it
>>>>>> removed here and this late in rc3 ? I think these patches should
>>>>>> really be postponed until after the release.  
>>>>>
>>>>> Hello Marek,  
>>>>
>>>> Hi,
>>>>  
>>>>> do you see a problem in principal with the removal of UPDATE_TFTP
>>>>> which is redundant to what you can do with DFU or is it only the
>>>>> timing issue?  
>>>>
>>>> I don't see how it is redundant. The usecase I see is a fitImage
>>>> which contains the update fragments is applied with a single
>>>> command this way. I don't see a suitable replacement.
>>>>  
>>>
>>> Hello Marek,
>>>
>>> CONFIG_UPDATE_TFTP=y does not support any command except the dfu
>>> tftp command which is not changed by this patch.
>>>
>>> CONFIG_UPDATE_TFTP=y further activates updating NOR flash by reading
>>> from a tftp server on every boot without any user control. Other
>>> target media are not supported. This is what is removed by the
>>> patch. And this is what can be replaced using preboot.
>>>
>>> I could not find a single config that uses UPDATE_TFTP. So where is
>>> this automated update of NOR flash really used?  
>>
>> I have it enabled on boards where it cannot be enabled upstream (for
>> various reasons), the following is enabled there:
>>
>> CONFIG_CMD_DFU=y
>> CONFIG_CMD_FITUPD=y
>> CONFIG_DFU_RAM=y
>> CONFIG_DFU_TFTP=y
>> CONFIG_UPDATE_TFTP=y
> 
> Marek, could you share those reasons? And why above CONFIG* options
> cannot be set in the upstream?

Platform security, U-Boot isn't the update mechanism there, but it's
convenient to use U-Boot as update mechanism during development (RCar3).

> It is the often practice to grep sources to look for "dead" configs
> (i.e. those which are not referenced on any board) and then remove
> features on this basis.

You might end up removing functionality which is useful to people this way.