lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 10 Apr 2018 19:23:08 -0700 From: Stephen Boyd <swboyd@...omium.org> To: Lina Iyer <ilina@...eaurora.org> Cc: andy.gross@...aro.org, david.brown@...aro.org, linux-arm-msm@...r.kernel.org, linux-soc@...r.kernel.org, rnayak@...eaurora.org, bjorn.andersson@...aro.org, linux-kernel@...r.kernel.org, evgreen@...omium.org, dianders@...omium.org Subject: Re: [PATCH v5 04/10] drivers: qcom: rpmh: add RPMH helper functions Quoting Lina Iyer (2018-04-09 08:36:31) > On Fri, Apr 06 2018 at 19:21 -0600, Stephen Boyd wrote: > >Quoting Lina Iyer (2018-04-05 09:18:28) > >> diff --git a/include/soc/qcom/rpmh.h b/include/soc/qcom/rpmh.h > >> new file mode 100644 > >> index 000000000000..95334d4c1ede > >> --- /dev/null > >> +++ b/include/soc/qcom/rpmh.h > >> @@ -0,0 +1,34 @@ > >> +/* SPDX-License-Identifier: GPL-2.0 */ > >> +/* > >> + * Copyright (c) 2016-2018, The Linux Foundation. All rights reserved. > >> + */ > >> + > >> +#ifndef __SOC_QCOM_RPMH_H__ > >> +#define __SOC_QCOM_RPMH_H__ > >> + > >> +#include <soc/qcom/tcs.h> > >> +#include <linux/platform_device.h> > >> + > >> +struct rpmh_client; > >> + > >> +#if IS_ENABLED(CONFIG_QCOM_RPMH) > >> +int rpmh_write(struct rpmh_client *rc, enum rpmh_state state, > >> + const struct tcs_cmd *cmd, u32 n); > >> + > >> +struct rpmh_client *rpmh_get_client(struct platform_device *pdev); > >> + > >> +void rpmh_release(struct rpmh_client *rc); > > > >Please get rid of this 'client' layer and fold it into the rpmh driver. > >Everything that uses the rpmh_client is a child device of the rpmh > >device so they should be able to just pass in their device pointer as > >their 'handle' and have the rpmh driver take that, get the parent device > >pointer, and pull an rpmh_drv structure out of there. The 'common' code > >can go into the base rpmh driver and get used from there and then we > >don't have to hop between two files to see how rpmh is used by the > >consumers. Code complexity goes down this way. > > That would be not be a good idea. This layer is not just providing an > API interface. There is resource buffering, handling of memory for > requests and downstream quirks and debug going on in this layer. It > would be unwise to clobber the hardware centric rpmh-rsc layer. If you > look at the series as a whole, you would understand why this is > necessary. I plan to build more on top of these patches in the future as > we add support for system low power modes. The complexity doesn't go > away, it just thrown in to another file, which is already decently > sized. > > I could try to use the device as a handle, and internally work on > getting the drv and other information from it, if that helps. But I do > not want to clobber these two files together. It doesn't help > maintainability. Using the device as a handle is a good start. Let's see how it looks once that part of the code gets replaced. I still fail to see how buffer management and requests are any different from poking the hardware, but OK. Maybe if this was a TCS "library" on top of the rpmh hardware interface?
Powered by blists - more mailing lists