Hi Michael,
> Sorry, I put this in but then hit the build break, I was going to fix it up
> but
> would rather you did and tested it, so we may as well do another review :)
whee!
>> @@ -0,0 +1,58 @@
>> +/*
>> + * OPAL Runtime Diagnostics interface driver
>> + * Supported on POWERNV platform
>> + *
>> + * (C) Copyright IBM 2015
>
> Usual syntax is: "Copyright IBM Corporation 2015"
OK, fixed.
>> + *
>> + * Author: Vaidyanathan Srinivasan <svaidy at linux.vnet.ibm.com>
>> + * Author: Jeremy Kerr <j...@ozlabs.org>
>
> I'd rather you dropped these, they'll just bit rot, but if you insist I don't
> care that much.
Yep, I'd rather remove them too.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2, or (at your option)
>> + * any later version.
>
> As pointed out by Daniel, we should probably be using the "version 2" only
> language on new files.
Fixed.
>> + vma->vm_page_prot = phys_mem_access_prot(file, vma->vm_pgoff,
>> + size, vma->vm_page_prot)
>> + | _PAGE_SPECIAL;
>
> This doesn't build with CONFIG_STRICT_MM_TYPECHECKS=y:
>
> arch/powerpc/platforms/powernv/opal-prd.c:131:5: error: invalid operands to
> binary | (have ‘pgprot_t’ and ‘int’)
> | _PAGE_SPECIAL;
OK, new patch coming with the proper pgprot macros.
>> + switch(cmd) {
> ^
> space please
Fixed.
>> + pr_devel("ioctl SCOM_READ: chip %llx addr %016llx "
>> + "data %016llx rc %lld\n",
>
> Don't split the string please.
OK, but this makes our lines >80 chars. Assuming that'll be okay.
>> +struct file_operations opal_prd_fops = {
>
> This can be static const I think.
Indeed it can! Updated.
>> +static struct miscdevice opal_prd_dev = {
>> + .minor = MISC_DYNAMIC_MINOR,
>> + .name = "opal-prd",
>> + .fops = &opal_prd_fops,
>
> White space is messed up here, should be leading tabs.
[tabs-spaces-both.png]
Thanks for the review, new patch coming soon.
Cheers,
Jeremy
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
