Re: [PATCH v3 03/10] drivers/peci: Add support for PECI bus driver core

[Jae Hyun Yoo]

On 4/24/2018 9:01 AM, Andy Shevchenko wrote:

On Tue, 2018-04-10 at 11:32 -0700, Jae Hyun Yoo wrote:

This commit adds driver implementation for PECI bus core into linux
driver framework.



All comments you got for patch 6 are applicable here.

And perhaps in the rest of the series.

The rule of thumb: when you get even single comment in a certain place,
re-check _entire_ series for the same / similar patterns!



Thanks for your advice. I'll keep that in mind.

Jae
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 03/10] drivers/peci: Add support for PECI bus driver core

[Andy Shevchenko]

On Tue, 2018-04-10 at 11:32 -0700, Jae Hyun Yoo wrote:
> This commit adds driver implementation for PECI bus core into linux
> driver framework.
> 

All comments you got for patch 6 are applicable here.

And perhaps in the rest of the series.

The rule of thumb: when you get even single comment in a certain place,
re-check _entire_ series for the same / similar patterns!

-- 
Andy Shevchenko 
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 03/10] drivers/peci: Add support for PECI bus driver core

[Jae Hyun Yoo]

On 4/23/2018 3:52 AM, Greg KH wrote:

On Tue, Apr 10, 2018 at 11:32:05AM -0700, Jae Hyun Yoo wrote:

+static void peci_adapter_dev_release(struct device *dev)
+{
+   /* do nothing */
+}


As per the in-kernel documentation, I am now allowed to make fun of you.

You are trying to "out smart" the kernel by getting rid of a warning
message that was explicitly put there for you to do something.  To think
that by just providing an "empty" function you are somehow fulfilling
the API requirement is quite bold, don't you think?

This has to be fixed.  I didn't put that warning in there for no good
reason.  Please go read the documentation again...

greg k-h



Hi Greg,

Thanks a lot for your review.

I think, it should contain actual device resource release code which is
being done by peci_del_adapter(), or a coupling logic should be added
between peci_adapter_dev_release() and peci_del_adapter().

As you suggested, I'll check it again after reading documentation and
understanding core.c code more deeply.

Jae
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 03/10] drivers/peci: Add support for PECI bus driver core

[kbuild test robot]

Hi Jae,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.17-rc1 next-20180419]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Jae-Hyun-Yoo/PECI-device-driver-introduction/20180411-180018
config: x86_64-randconfig-s0-04192349 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers//peci/peci-core.c: In function 'peci_device_match':
>> drivers//peci/peci-core.c:739:6: error: implicit declaration of function 
>> 'peci_of_match_device' [-Werror=implicit-function-declaration]
 if (peci_of_match_device(drv->of_match_table, client))
 ^~~~
   At top level:
   drivers//peci/peci-core.c:840:28: warning: 'peci_new_device' defined but not 
used [-Wunused-function]
static struct peci_client *peci_new_device(struct peci_adapter *adapter,
   ^~~
   drivers//peci/peci-core.c:135:29: warning: 'peci_verify_adapter' defined but 
not used [-Wunused-function]
static struct peci_adapter *peci_verify_adapter(struct device *dev)
^~~
   cc1: some warnings being treated as errors

vim +/peci_of_match_device +739 drivers//peci/peci-core.c

   732  
   733  static int peci_device_match(struct device *dev, struct device_driver 
*drv)
   734  {
   735  struct peci_client *client = peci_verify_client(dev);
   736  struct peci_driver *driver;
   737  
   738  /* Attempt an OF style match */
 > 739  if (peci_of_match_device(drv->of_match_table, client))
   740  return 1;
   741  
   742  driver = to_peci_driver(drv);
   743  
   744  if (peci_match_id(driver->id_table, client))
   745  return 1;
   746  
   747  return 0;
   748  }
   749  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip