[U-Boot] IS_ERR_VALUE failing on socfpga gen5

[Simon Goldschmidt]

Am 14.10.2019 um 22:02 schrieb Simon Glass:
> Hi Simon,
> 
> On Mon, 14 Oct 2019 at 13:13, Simon Goldschmidt
> <simon.k.r.goldschmidt at gmail.com> wrote:
>>
>> Am 12.10.2019 um 00:11 schrieb Simon Glass:
>>> Hi Simon,
>>>
>>> On Fri, 11 Oct 2019 at 12:31, Simon Goldschmidt
>>> <simon.k.r.goldschmidt at gmail.com> wrote:
>>>>
>>>>
>>>>
>>>> Simon Glass <sjg at chromium.org> schrieb am Fr., 11. Okt. 2019, 20:27:
>>>>>
>>>>> Hi Simon,
>>>>>
>>>>> On Tue, 8 Oct 2019 at 14:34, Simon Goldschmidt
>>>>> <simon.k.r.goldschmidt at gmail.com> wrote:
>>>>>>
>>>>>> In a series I'm currently preparing, I've stumbled accross the fact that
>>>>>> IS_ERR_VALUE() doesn't reliably work on socfpga SPL as the onchip SRAM
>>>>>> begins at 0xffff0000 and the heap is at the end of the 32 bit range.
>>>>>>
>>>>>
>>>>> Which function is returning that address?
>>>>
>>>>
>>>> Sorry, I don't know right now and it was kind of hard to debug. But it was somewhere in the DM initialization of new drivers I was adding. One of those functions returned a heap pointer that was interpreted as an error due to the address being in the range regarded as "error pointer"...
>>>
>>> I don't know a good solution to this. But we should consider changing
>>> whatever API it was to use an int return value instead of an address.
>>> The address can be passed back in another parameter. That is how most
>>> uclass methods work.
>>
>> Digging into this again, it was 'syscon_node_to_regmap', which seems to
>> be copied from Linux, so I don't think it's feasible to change this
>> function. Also, IS_ERR() is used in ~120 files, I wouldn't want to
>> change all these for socfpga.
>>
>> One idea that came to mind was to change the ERR_PTR/PTR_ERR macros in
>> linux/err.h to convert errors to an architecture-specified pointer range
>> instead of just converting the data type. However, that would mean
>> another change to Linux imports, and Tom just seemed a little opposed to
>> that...?
> 
> I think we should keep it simple and change syscon_node_to_regmap() to
> return an int. It's an easy change affecting 17 files and avoids the
> problem.

Well, it took me some hours to find the issue (without access to a 
debugger, sadly) and I'd like to save me and others from facing this 
again in the future. When I write the next driver, who tells me 
internals aren't using IS_ERR again?

Plus, linux/err.h says:

"This should be a per-architecture thing, to allow different error and 
pointer decisions."

Let me prepare a patch to show what I mean and we can see if it's 
acceptable.

Regards,
Simon

> 
>>
>> Tom?
> 
> Regards,
> Simon
>