| Message ID | 1472226597-20862-1-git-send-email-l.stach@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Aug 26, 2016 at 05:49:54PM +0200, Lucas Stach wrote: > The devicetree documentation states that those are required properties, > so the driver should refuse to probe if those are absent to be > consistent. This will also allow to drop some error checking from the > clock enable/disable paths. NAK. Thanks for reviewing the existing DT files before proposing this change and noticing that you're going to wilfully end up breaking existing users. A simple grep would have sufficed. The DT binding doc is wrong: there is only one documented clock on Dove and that's for the GPU core. (The Dove documentation as far as clocks go is very poor.) So, what's Dove supposed to do - make up some ficticious clock?
Am Freitag, den 26.08.2016, 17:10 +0100 schrieb Russell King - ARM Linux: > On Fri, Aug 26, 2016 at 05:49:54PM +0200, Lucas Stach wrote: > > The devicetree documentation states that those are required properties, > > so the driver should refuse to probe if those are absent to be > > consistent. This will also allow to drop some error checking from the > > clock enable/disable paths. > > NAK. > > Thanks for reviewing the existing DT files before proposing this change > and noticing that you're going to wilfully end up breaking existing users. > A simple grep would have sufficed. > Gah, thanks for pointing this out. > The DT binding doc is wrong: there is only one documented clock on Dove > and that's for the GPU core. (The Dove documentation as far as clocks > go is very poor.) So, what's Dove supposed to do - make up some > ficticious clock? > Core, bus and shader are all module input clocks. If the SoC integration provides the same clock for all inputs, the DT should reflect this by supplying the same clock for all 3 inputs. I'm going to change this patch to keep things working for the Dove DTs, but I think they really should be changed to supply all 3 input clocks. I'm sorry for not noticing this when you proposed the Dove GPU DT support. Regards, Lucas
On Mon, Aug 29, 2016 at 12:47:20PM +0200, Lucas Stach wrote: > Core, bus and shader are all module input clocks. If the SoC integration > provides the same clock for all inputs, the DT should reflect this by > supplying the same clock for all 3 inputs. You're making an assertion that we don't know is true. There is no evidence that the GC600 has three input clocks. Just because iMX Vivante GPUs have three clocks does not mean that all Vivante IP has three clock inputs.
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c index b851809d29b3..ec14aaaf6dd7 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c @@ -1684,12 +1684,12 @@ static int etnaviv_gpu_platform_probe(struct platform_device *pdev) gpu->clk_bus = devm_clk_get(&pdev->dev, "bus"); DBG("clk_bus: %p", gpu->clk_bus); if (IS_ERR(gpu->clk_bus)) - gpu->clk_bus = NULL; + return PTR_ERR(gpu->clk_bus); gpu->clk_core = devm_clk_get(&pdev->dev, "core"); DBG("clk_core: %p", gpu->clk_core); if (IS_ERR(gpu->clk_core)) - gpu->clk_core = NULL; + return PTR_ERR(gpu->clk_core); gpu->clk_shader = devm_clk_get(&pdev->dev, "shader"); DBG("clk_shader: %p", gpu->clk_shader);
The devicetree documentation states that those are required properties, so the driver should refuse to probe if those are absent to be consistent. This will also allow to drop some error checking from the clock enable/disable paths. Signed-off-by: Lucas Stach <l.stach@pengutronix.de> --- drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)