On Thu, Dec 08, 2016 at 09:04:13PM +0800, Grazvydas Ignotas wrote: > On Thu, Dec 8, 2016 at 11:50 AM, Huang Rui <ray.huang at amd.com> wrote: > > On Thu, Dec 08, 2016 at 05:27:30PM +0800, Christian König wrote: > >> Am 08.12.2016 um 10:02 schrieb Huang Rui: > >> > On Thu, Dec 08, 2016 at 04:41:04PM +0800, Koenig, Christian wrote: > >> >> Sorry, but that just sounds like OS abstraction code which isn't allowed. > >> >> > >> >> There is no benefit except routing all messages through CGS which makes > >> >> things much harder to follow. > >> >> > >> > There isn't COS part at current driver. But it seems to be not good to > >> > introduce COS just for prints. Actually, most of drivers prefer to use > >> > dev_* prints, and it's able to dynamic control the print level when we > >> > debug it. > >> > >> Well I'm not sure if you have understood what I wanted to say. > >> > >> The reason that there isn't any COS abstraction is that it isn't allowed > >> upstream. > >> > > > > OK, I see. > > > >> Using the dev_* prints in the powerplay code is fine, but don't use the > >> CGS or any other abstraction layer for them. > >> > > > > Powerplay is quite independent component without amdgpu object, it is > > hard to use dev_* prints without any abstraction layer. > > Maybe you could use dev_set_name() with something powerplay related on > relevant devices and then dev_* will print what you want? > Garzvydas, thanks to your comments. dev_set_name function needs "struct device" object in powerplay. But powerplay uses CGS layer to call into "amdgpu" or "struct device". I think the intention is to make code porting easier. So we won't bring any OS abstraction code into powerplay. > Alternatively you could do > > #define pr_fmt(fmt) "[powerplay] " fmt > > before #include <linux/...> and then all pr_* functions will prefix > their messages. > pr_* actually is almost the same with printk, but it seems fine here to just add a prefix. Thanks, Rui