[lkml]   [2015]   [Jan]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH 2/2] platform/x86: Add Intel Galileo platform specific setup
On 09/01/15 01:00, Ong, Boon Leong wrote:

>> +static void __init
>> +intel_galileo_imr_sanity(unsigned long base, unsigned long size) {
>> + /* Test zero zero */
>> + if (imr_add(0, 0, 0, 0, true) == 0)
>> + pr_err(SANITY "zero sized IMR @ 0x00000000\n");
> A side-discussion on imr_add():
> I would think that we should allow 1KiB IMR setting. Current imr_add() logic
> is prohibiting it.

Hi Boon Leong. Ermm, it does allow 1 KiB IMR regions. The following code
works on the unmodifed V1 driver.

/* Test 1 KiB works */
idx = imr_add(0, IMR_ALIGN, IMR_READ_ACCESS_ALL,
if (idx < 0)
pr_err(SANITY "Couldn't add an IMR @ 0x%04x bytes\n", IMR_ALIGN);

Note IMR_ALIGN is 0x400

I'll add that test to the set of sanity tests in V2 just to put your
mind at ease though.

> So, the 'size' input should be at least 1KiB and imr_add()
> internal logic will adjust 'hi' by -1KiB. Please consider ..


Actually I had a response all typed out for you why I didn't want an API
to presume to modify the size of my input from the user's POV but,
thinking about it twice - I agree with you.

V2 will subtract IMR_ALIGN (0x400) bytes from the size.

It's stupid to have to subtract IMR_ALIGN bytes on the input - and
assumes the user of the API understands how the hardware works - but, of
course the point of an API is so that the user of it doesn't *have* to
understand that.

Good call.


 \ /
  Last update: 2015-01-09 03:21    [W:0.129 / U:0.212 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site