From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-oi1-f175.google.com (mail-oi1-f175.google.com [209.85.167.175]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A148C1D0940 for ; Mon, 14 Oct 2024 21:14:08 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.167.175 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1728940450; cv=none; b=mtekKDGpl6+/nNz7o38C5myWRmnz8Zp0KOqYODoY4G2ibXsLSMw0sLVzpzWBaeIGlSCu99CGWKU2F7toJx+fUbOTTTBmE+4qLd7YLwIHyzu4l6y7z1266CFsgP9tRxyo+fs5WfJ2ROITh5ptgDej1IBvd4f3HQ2h4Y5jl9wvmXU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1728940450; c=relaxed/simple; bh=NNULu7V4/uFQOOw1zu9eVvMWDZPrUoD1MFk/IRT5hLU=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=UlNfcrxNpxu8I+Tu5Hxah6kDbcvoSoT3proFYUGLWxA+q1YbCnpFIFD+NwW8d5aIkLJWohZDgrgTYIHGWH40yEcQyy480dday7i54uRC0DutmxP+1uqxaALVn/P4nKB5LnCllZ2HMLzkyH0YK9hQbQtg3ZeQbwFQnIqt6gsA4yI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=baylibre.com; spf=pass smtp.mailfrom=baylibre.com; dkim=pass (2048-bit key) header.d=baylibre-com.20230601.gappssmtp.com header.i=@baylibre-com.20230601.gappssmtp.com header.b=wV8lYQSL; arc=none smtp.client-ip=209.85.167.175 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=baylibre.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=baylibre.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=baylibre-com.20230601.gappssmtp.com header.i=@baylibre-com.20230601.gappssmtp.com header.b="wV8lYQSL" Received: by mail-oi1-f175.google.com with SMTP id 5614622812f47-3e5c9295f3aso897470b6e.1 for ; Mon, 14 Oct 2024 14:14:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre-com.20230601.gappssmtp.com; s=20230601; t=1728940448; x=1729545248; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=zq14q1R1F8s50ju9k71p1ZwPAO4Av1bykZb7U4Dj1cE=; b=wV8lYQSLveiIsyBaD9d01kQXbU3B+KqPSMv2Odc571NxUsY/JunsWZSs749976PW/t ll3pCd6M+BJRSp8XBJZohIxKdXD1akV3h3Ynml8mMwvMfL1TB8X+ZNb1oAX47tCAh5k0 tDyWg2FgUjq7eltPyg/xnBBvJE1txW71D5x54ZQBQ1iw3PMpJteqwz9BQ/lDrFyv5Mfj yE6BwqUmWAkvvCZ/B64/duZqTOdfEQH8F0XxGDbvv6wl9rX26XtQpjpKm71N9vkBO2kz O5lUypYrNgO731RkwzgeQRnAeP7ExVznivds6vxg7KmI5LmYShktBpVF32EEuRywK7WU WOUg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1728940448; x=1729545248; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=zq14q1R1F8s50ju9k71p1ZwPAO4Av1bykZb7U4Dj1cE=; b=f9SrTZ+KGir7FnJCbwci06cwVEAewrTkplYRwpGPOJpfHKvo5OoLEsC9cTLza0nQbg SnbvrrJIsOIOahltWk8FXUeci2Ue3XVcPVryvygUbYQGIRFY1+rsVmBhuJW1H883+ODl cZ27PJFtsiXX0BhNTgca+95AI4ITQ6/5EMiiryojU4TWy5khPmorkb/mve5XLm+gkprJ PGboGt1l3eiwddKPtWEKQC8225kgbdrVTCPulqkJNAox6pZSdHIgEOY06AZ39LNZ99rT yOe2400gPc1fK0Xdj44El6vLpXCekVlXnQE3i0DGc//5Ipv0p8PLbVBK1T++YAI/OoeO yULw== X-Forwarded-Encrypted: i=1; AJvYcCWxSp35TFV/htj7o2ucBju7hak4hinn6PskW+pG6QFpbeSI3AF//w/JDvBR5jCPPrskVD9oqzJQz19z@vger.kernel.org X-Gm-Message-State: AOJu0YzhYzjPx0Lq2qaecIxAFn7YVi9QOJSTD/Klos1fN90ldOn1J0kR MMqlkoIhq5xqPuxNxeSYVD10dRX/eLhze6EDMmcF81Jh5KG8C65vfXyZt3Jg5zs= X-Google-Smtp-Source: AGHT+IFBQpcdVQJpDxZ/TlLOQl91w5TGGNDrZiZmWC5MClEg9c6QlPh8jb8yUjuS+qoWYS4wRLVRcg== X-Received: by 2002:a05:6870:3749:b0:27d:10f5:347 with SMTP id 586e51a60fabf-28887343aa1mr4749601fac.15.1728940447653; Mon, 14 Oct 2024 14:14:07 -0700 (PDT) Received: from [192.168.0.142] (ip98-183-112-25.ok.ok.cox.net. [98.183.112.25]) by smtp.gmail.com with ESMTPSA id 586e51a60fabf-288581dcebbsm2957223fac.27.2024.10.14.14.14.04 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 14 Oct 2024 14:14:06 -0700 (PDT) Message-ID: Date: Mon, 14 Oct 2024 16:14:04 -0500 Precedence: bulk X-Mailing-List: devicetree@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v6 4/8] iio: dac: adi-axi-dac: extend features To: Angelo Dureghello , =?UTF-8?Q?Nuno_S=C3=A1?= , Lars-Peter Clausen , Michael Hennerich , Jonathan Cameron , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Olivier Moysan Cc: linux-iio@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Mark Brown References: <20241014-wip-bl-ad3552r-axi-v0-iio-testing-v6-0-eeef0c1e0e56@baylibre.com> <20241014-wip-bl-ad3552r-axi-v0-iio-testing-v6-4-eeef0c1e0e56@baylibre.com> Content-Language: en-US From: David Lechner In-Reply-To: <20241014-wip-bl-ad3552r-axi-v0-iio-testing-v6-4-eeef0c1e0e56@baylibre.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 10/14/24 5:08 AM, Angelo Dureghello wrote: > From: Angelo Dureghello > > Extend AXI-DAC backend with new features required to interface > to the ad3552r DAC. Mainly, a new compatible string is added to > support the ad3552r-axi DAC IP, very similar to the generic DAC > IP but with some customizations to work with the ad3552r. > > Then, a serie of generic functions has been added to match with spelling: series > ad3552r needs. Function names has been kept generic as much as > possible, to allow re-utilization from other frontend drivers. > > Signed-off-by: Angelo Dureghello > --- ... > +static int axi_dac_read_raw(struct iio_backend *back, > + struct iio_chan_spec const *chan, > + int *val, int *val2, long mask) > +{ > + struct axi_dac_state *st = iio_backend_get_priv(back); > + int err, reg; > + > + switch (mask) { > + case IIO_CHAN_INFO_FREQUENCY: > + > + if (!st->info->has_dac_clk) > + return -EOPNOTSUPP; > + > + /* > + * As from ad3552r AXI IP documentation, > + * returning the SCLK depending on the stream mode. > + */ > + err = regmap_read(st->regmap, AXI_DAC_CUSTOM_CTRL_REG, ®); > + if (err) > + return err; > + > + if (reg & AXI_DAC_CUSTOM_CTRL_STREAM) > + *val = st->dac_clk_rate / 2; > + else > + *val = st->dac_clk_rate / 8; To get the DAC sample rate, we only care about the streaming mode rate, so this should just always be / 2 and not / 8. Otherwise the sampling_frequency attribute in the DAC driver will return the wrong value when the buffer is not enabled. We never do buffered writes without enabling streaming mode. > + > + return IIO_VAL_INT; > + default: > + return -EINVAL; > + } > +} > + > +static int axi_dac_bus_reg_write(struct iio_backend *back, u32 reg, u32 val, > + size_t data_size) > +{ > + struct axi_dac_state *st = iio_backend_get_priv(back); > + int ret; > + u32 ival; > + > + if (data_size == sizeof(u16)) > + ival = FIELD_PREP(AXI_DAC_CUSTOM_WR_DATA_16, val); > + else > + ival = FIELD_PREP(AXI_DAC_CUSTOM_WR_DATA_8, val); > + > + ret = regmap_write(st->regmap, AXI_DAC_CUSTOM_WR_REG, ival); > + if (ret) > + return ret; > + > + /* > + * Both REG_CNTRL_2 and AXI_DAC_CNTRL_DATA_WR need to know I'm guessing these got renamed. REG_CNTRL_2 = AXI_DAC_CNTRL_2_REG and AXI_DAC_CNTRL_DATA_WR = AXI_DAC_CUSTOM_WR_REG? > + * the data size. So keeping data size control here only, > + * since data size is mandatory for the current transfer. > + * DDR state handled separately by specific backend calls, > + * generally all raw register writes are SDR. > + */ > + if (data_size == sizeof(u8)) > + ret = regmap_set_bits(st->regmap, AXI_DAC_CNTRL_2_REG, > + AXI_DAC_CNTRL_2_SYMB_8B); > + else > + ret = regmap_clear_bits(st->regmap, AXI_DAC_CNTRL_2_REG, > + AXI_DAC_CNTRL_2_SYMB_8B); > + if (ret) > + return ret; > + > + ret = regmap_update_bits(st->regmap, AXI_DAC_CUSTOM_CTRL_REG, > + AXI_DAC_CUSTOM_CTRL_ADDRESS, > + FIELD_PREP(AXI_DAC_CUSTOM_CTRL_ADDRESS, reg)); > + if (ret) > + return ret; > + > + ret = regmap_update_bits(st->regmap, AXI_DAC_CUSTOM_CTRL_REG, > + AXI_DAC_CUSTOM_CTRL_TRANSFER_DATA, > + AXI_DAC_CUSTOM_CTRL_TRANSFER_DATA); > + if (ret) > + return ret; > + > + ret = regmap_read_poll_timeout(st->regmap, > + AXI_DAC_CUSTOM_CTRL_REG, ival, > + ival & AXI_DAC_CUSTOM_CTRL_TRANSFER_DATA, > + 10, 100 * KILO); > + if (ret) > + return ret; Should we also clear AXI_DAC_CUSTOM_CTRL_TRANSFER_DATA on timeout so that we don't leave things in a bad state? > + > + return regmap_clear_bits(st->regmap, AXI_DAC_CUSTOM_CTRL_REG, > + AXI_DAC_CUSTOM_CTRL_TRANSFER_DATA); > +} > + ... > static int axi_dac_probe(struct platform_device *pdev) > { > - const unsigned int *expected_ver; > struct axi_dac_state *st; > void __iomem *base; > unsigned int ver; > @@ -566,15 +793,26 @@ static int axi_dac_probe(struct platform_device *pdev) > if (!st) > return -ENOMEM; > > - expected_ver = device_get_match_data(&pdev->dev); > - if (!expected_ver) > + st->info = device_get_match_data(&pdev->dev); > + if (!st->info) > return -ENODEV; > > - clk = devm_clk_get_enabled(&pdev->dev, NULL); > + clk = devm_clk_get_enabled(&pdev->dev, "s_axi_aclk"); This will break existing users that don't have clock-names in the DT. It should be fine to leave it as NULL in which case it will get the clock at index 0 in the clocks array even if there is more than one clock. > if (IS_ERR(clk)) > return dev_err_probe(&pdev->dev, PTR_ERR(clk), > "failed to get clock\n"); > > + if (st->info->has_dac_clk) { > + struct clk *dac_clk; > + > + dac_clk = devm_clk_get_enabled(&pdev->dev, "dac_clk"); > + if (IS_ERR(dac_clk)) > + return dev_err_probe(&pdev->dev, PTR_ERR(dac_clk), > + "failed to get dac_clk clock\n"); > + > + st->dac_clk_rate = clk_get_rate(dac_clk); > + } > + > base = devm_platform_ioremap_resource(pdev, 0); > if (IS_ERR(base)) > return PTR_ERR(base);