| Message ID | 20241027091440.1913863-2-csokas.bence@prolan.hu (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v2,01/10] dma-engine: sun4i: Add a quirk to support different chips | expand |
On 2024. 10. 27. 10:14, Csókás, Bence wrote: > From: Mesih Kilinc <mesihkilinc@gmail.com> > > Allwinner suniv F1C100s has a reset bit for DMA in CCU. Sun4i do not > has this bit but in order to support suniv we need to add it. So add > support for reset bit. > > Signed-off-by: Mesih Kilinc <mesihkilinc@gmail.com> > [ csokas.bence: Rebased and addressed comments ] > Signed-off-by: Csókás, Bence <csokas.bence@prolan.hu> > --- > > Notes: > Changes in v2: > * Call reset_control_deassert() unconditionally, as it supports optional resets > * Use dev_err_probe() I missed one, namely: > + dev_err(&pdev->dev, > + "Failed to deassert the reset control\n"); > + goto err_clk_disable; > + } For now I'll resubmit just this patch, and then wait for more comments that may arise during the week, then resubmit the whole amended series.
Hi Bence, kernel test robot noticed the following build errors: [auto build test ERROR on sunxi/sunxi/for-next] [also build test ERROR on vkoul-dmaengine/next broonie-sound/for-next arm64/for-next/core clk/clk-next kvmarm/next rockchip/for-next shawnguo/for-next soc/for-next arm/for-next arm/fixes linus/master v6.12-rc4 next-20241025] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Cs-k-s-Bence/dma-engine-sun4i-Add-has_reset-option-to-quirk/20241027-172307 base: https://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git sunxi/for-next patch link: https://lore.kernel.org/r/20241027091440.1913863-2-csokas.bence%40prolan.hu patch subject: [PATCH v2 02/10] dma-engine: sun4i: Add has_reset option to quirk config: arm-multi_v7_defconfig (https://download.01.org/0day-ci/archive/20241028/202410280225.baqFmTsa-lkp@intel.com/config) compiler: arm-linux-gnueabi-gcc (GCC) 13.3.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241028/202410280225.baqFmTsa-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202410280225.baqFmTsa-lkp@intel.com/ All error/warnings (new ones prefixed by >>): drivers/dma/sun4i-dma.c: In function 'sun4i_dma_probe': >> drivers/dma/sun4i-dma.c:1225:51: warning: passing argument 2 of 'dev_err_probe' makes integer from pointer without a cast [-Wint-conversion] 1225 | dev_err_probe(&pdev->dev, "Failed to get reset control\n"); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | | | char * In file included from include/linux/device.h:15, from include/linux/dma-mapping.h:8, from drivers/dma/sun4i-dma.c:10: include/linux/dev_printk.h:278:64: note: expected 'int' but argument is of type 'char *' 278 | __printf(3, 4) int dev_err_probe(const struct device *dev, int err, const char *fmt, ...); | ~~~~^~~ >> drivers/dma/sun4i-dma.c:1225:25: error: too few arguments to function 'dev_err_probe' 1225 | dev_err_probe(&pdev->dev, "Failed to get reset control\n"); | ^~~~~~~~~~~~~ include/linux/dev_printk.h:278:20: note: declared here 278 | __printf(3, 4) int dev_err_probe(const struct device *dev, int err, const char *fmt, ...); | ^~~~~~~~~~~~~ vim +/dev_err_probe +1225 drivers/dma/sun4i-dma.c 1193 1194 static int sun4i_dma_probe(struct platform_device *pdev) 1195 { 1196 struct sun4i_dma_dev *priv; 1197 int i, j, ret; 1198 1199 priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); 1200 if (!priv) 1201 return -ENOMEM; 1202 1203 priv->cfg = of_device_get_match_data(&pdev->dev); 1204 if (!priv->cfg) 1205 return -ENODEV; 1206 1207 priv->base = devm_platform_ioremap_resource(pdev, 0); 1208 if (IS_ERR(priv->base)) 1209 return PTR_ERR(priv->base); 1210 1211 priv->irq = platform_get_irq(pdev, 0); 1212 if (priv->irq < 0) 1213 return priv->irq; 1214 1215 priv->clk = devm_clk_get(&pdev->dev, NULL); 1216 if (IS_ERR(priv->clk)) { 1217 dev_err(&pdev->dev, "No clock specified\n"); 1218 return PTR_ERR(priv->clk); 1219 } 1220 1221 if (priv->cfg->has_reset) { 1222 priv->rst = devm_reset_control_get_exclusive(&pdev->dev, 1223 NULL); 1224 if (IS_ERR(priv->rst)) { > 1225 dev_err_probe(&pdev->dev, "Failed to get reset control\n"); 1226 return PTR_ERR(priv->rst); 1227 } 1228 } 1229 1230 platform_set_drvdata(pdev, priv); 1231 spin_lock_init(&priv->lock); 1232 1233 dma_set_max_seg_size(&pdev->dev, SUN4I_DMA_MAX_SEG_SIZE); 1234 1235 dma_cap_zero(priv->slave.cap_mask); 1236 dma_cap_set(DMA_PRIVATE, priv->slave.cap_mask); 1237 dma_cap_set(DMA_MEMCPY, priv->slave.cap_mask); 1238 dma_cap_set(DMA_CYCLIC, priv->slave.cap_mask); 1239 dma_cap_set(DMA_SLAVE, priv->slave.cap_mask); 1240 1241 INIT_LIST_HEAD(&priv->slave.channels); 1242 priv->slave.device_free_chan_resources = sun4i_dma_free_chan_resources; 1243 priv->slave.device_tx_status = sun4i_dma_tx_status; 1244 priv->slave.device_issue_pending = sun4i_dma_issue_pending; 1245 priv->slave.device_prep_slave_sg = sun4i_dma_prep_slave_sg; 1246 priv->slave.device_prep_dma_memcpy = sun4i_dma_prep_dma_memcpy; 1247 priv->slave.device_prep_dma_cyclic = sun4i_dma_prep_dma_cyclic; 1248 priv->slave.device_config = sun4i_dma_config; 1249 priv->slave.device_terminate_all = sun4i_dma_terminate_all; 1250 priv->slave.copy_align = 2; 1251 priv->slave.src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) | 1252 BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) | 1253 BIT(DMA_SLAVE_BUSWIDTH_4_BYTES); 1254 priv->slave.dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) | 1255 BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) | 1256 BIT(DMA_SLAVE_BUSWIDTH_4_BYTES); 1257 priv->slave.directions = BIT(DMA_DEV_TO_MEM) | 1258 BIT(DMA_MEM_TO_DEV); 1259 priv->slave.residue_granularity = DMA_RESIDUE_GRANULARITY_BURST; 1260 1261 priv->slave.dev = &pdev->dev; 1262 1263 priv->pchans = devm_kcalloc(&pdev->dev, priv->cfg->dma_nr_max_channels, 1264 sizeof(struct sun4i_dma_pchan), GFP_KERNEL); 1265 priv->vchans = devm_kcalloc(&pdev->dev, SUN4I_DMA_NR_MAX_VCHANS, 1266 sizeof(struct sun4i_dma_vchan), GFP_KERNEL); 1267 priv->pchans_used = devm_kcalloc(&pdev->dev, 1268 BITS_TO_LONGS(priv->cfg->dma_nr_max_channels), 1269 sizeof(unsigned long), GFP_KERNEL); 1270 if (!priv->vchans || !priv->pchans || !priv->pchans_used) 1271 return -ENOMEM; 1272 1273 /* 1274 * [0..priv->cfg->ndma_nr_max_channels) are normal pchans, and 1275 * [priv->cfg->ndma_nr_max_channels..priv->cfg->dma_nr_max_channels) are 1276 * dedicated ones 1277 */ 1278 for (i = 0; i < priv->cfg->ndma_nr_max_channels; i++) 1279 priv->pchans[i].base = priv->base + 1280 SUN4I_NDMA_CHANNEL_REG_BASE(i); 1281 1282 for (j = 0; i < priv->cfg->dma_nr_max_channels; i++, j++) { 1283 priv->pchans[i].base = priv->base + 1284 SUN4I_DDMA_CHANNEL_REG_BASE(j); 1285 priv->pchans[i].is_dedicated = 1; 1286 } 1287 1288 for (i = 0; i < SUN4I_DMA_NR_MAX_VCHANS; i++) { 1289 struct sun4i_dma_vchan *vchan = &priv->vchans[i]; 1290 1291 spin_lock_init(&vchan->vc.lock); 1292 vchan->vc.desc_free = sun4i_dma_free_contract; 1293 vchan_init(&vchan->vc, &priv->slave); 1294 } 1295 1296 ret = clk_prepare_enable(priv->clk); 1297 if (ret) { 1298 dev_err(&pdev->dev, "Couldn't enable the clock\n"); 1299 return ret; 1300 } 1301 1302 /* Deassert the reset control */ 1303 ret = reset_control_deassert(priv->rst); 1304 if (ret) { 1305 dev_err(&pdev->dev, 1306 "Failed to deassert the reset control\n"); 1307 goto err_clk_disable; 1308 } 1309 1310 /* 1311 * Make sure the IRQs are all disabled and accounted for. The bootloader 1312 * likes to leave these dirty 1313 */ 1314 writel(0, priv->base + SUN4I_DMA_IRQ_ENABLE_REG); 1315 writel(0xFFFFFFFF, priv->base + SUN4I_DMA_IRQ_PENDING_STATUS_REG); 1316 1317 ret = devm_request_irq(&pdev->dev, priv->irq, sun4i_dma_interrupt, 1318 0, dev_name(&pdev->dev), priv); 1319 if (ret) { 1320 dev_err(&pdev->dev, "Cannot request IRQ\n"); 1321 goto err_clk_disable; 1322 } 1323 1324 ret = dma_async_device_register(&priv->slave); 1325 if (ret) { 1326 dev_warn(&pdev->dev, "Failed to register DMA engine device\n"); 1327 goto err_clk_disable; 1328 } 1329 1330 ret = of_dma_controller_register(pdev->dev.of_node, sun4i_dma_of_xlate, 1331 priv); 1332 if (ret) { 1333 dev_err(&pdev->dev, "of_dma_controller_register failed\n"); 1334 goto err_dma_unregister; 1335 } 1336 1337 dev_dbg(&pdev->dev, "Successfully probed SUN4I_DMA\n"); 1338 1339 return 0; 1340 1341 err_dma_unregister: 1342 dma_async_device_unregister(&priv->slave); 1343 err_clk_disable: 1344 clk_disable_unprepare(priv->clk); 1345 return ret; 1346 } 1347
Hi Bence, kernel test robot noticed the following build errors: [auto build test ERROR on sunxi/sunxi/for-next] [also build test ERROR on vkoul-dmaengine/next broonie-sound/for-next arm64/for-next/core clk/clk-next kvmarm/next rockchip/for-next shawnguo/for-next soc/for-next arm/for-next arm/fixes linus/master v6.12-rc4 next-20241025] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Cs-k-s-Bence/dma-engine-sun4i-Add-has_reset-option-to-quirk/20241027-172307 base: https://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git sunxi/for-next patch link: https://lore.kernel.org/r/20241027091440.1913863-2-csokas.bence%40prolan.hu patch subject: [PATCH v2 02/10] dma-engine: sun4i: Add has_reset option to quirk config: arm-sunxi_defconfig (https://download.01.org/0day-ci/archive/20241028/202410280330.S1S4TKbz-lkp@intel.com/config) compiler: arm-linux-gnueabi-gcc (GCC) 14.1.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241028/202410280330.S1S4TKbz-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202410280330.S1S4TKbz-lkp@intel.com/ All errors (new ones prefixed by >>): drivers/dma/sun4i-dma.c: In function 'sun4i_dma_probe': >> drivers/dma/sun4i-dma.c:1225:51: error: passing argument 2 of 'dev_err_probe' makes integer from pointer without a cast [-Wint-conversion] 1225 | dev_err_probe(&pdev->dev, "Failed to get reset control\n"); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | | | char * In file included from include/linux/device.h:15, from include/linux/dma-mapping.h:8, from drivers/dma/sun4i-dma.c:10: include/linux/dev_printk.h:278:64: note: expected 'int' but argument is of type 'char *' 278 | __printf(3, 4) int dev_err_probe(const struct device *dev, int err, const char *fmt, ...); | ~~~~^~~ drivers/dma/sun4i-dma.c:1225:25: error: too few arguments to function 'dev_err_probe' 1225 | dev_err_probe(&pdev->dev, "Failed to get reset control\n"); | ^~~~~~~~~~~~~ include/linux/dev_printk.h:278:20: note: declared here 278 | __printf(3, 4) int dev_err_probe(const struct device *dev, int err, const char *fmt, ...); | ^~~~~~~~~~~~~ vim +/dev_err_probe +1225 drivers/dma/sun4i-dma.c 1193 1194 static int sun4i_dma_probe(struct platform_device *pdev) 1195 { 1196 struct sun4i_dma_dev *priv; 1197 int i, j, ret; 1198 1199 priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); 1200 if (!priv) 1201 return -ENOMEM; 1202 1203 priv->cfg = of_device_get_match_data(&pdev->dev); 1204 if (!priv->cfg) 1205 return -ENODEV; 1206 1207 priv->base = devm_platform_ioremap_resource(pdev, 0); 1208 if (IS_ERR(priv->base)) 1209 return PTR_ERR(priv->base); 1210 1211 priv->irq = platform_get_irq(pdev, 0); 1212 if (priv->irq < 0) 1213 return priv->irq; 1214 1215 priv->clk = devm_clk_get(&pdev->dev, NULL); 1216 if (IS_ERR(priv->clk)) { 1217 dev_err(&pdev->dev, "No clock specified\n"); 1218 return PTR_ERR(priv->clk); 1219 } 1220 1221 if (priv->cfg->has_reset) { 1222 priv->rst = devm_reset_control_get_exclusive(&pdev->dev, 1223 NULL); 1224 if (IS_ERR(priv->rst)) { > 1225 dev_err_probe(&pdev->dev, "Failed to get reset control\n"); 1226 return PTR_ERR(priv->rst); 1227 } 1228 } 1229 1230 platform_set_drvdata(pdev, priv); 1231 spin_lock_init(&priv->lock); 1232 1233 dma_set_max_seg_size(&pdev->dev, SUN4I_DMA_MAX_SEG_SIZE); 1234 1235 dma_cap_zero(priv->slave.cap_mask); 1236 dma_cap_set(DMA_PRIVATE, priv->slave.cap_mask); 1237 dma_cap_set(DMA_MEMCPY, priv->slave.cap_mask); 1238 dma_cap_set(DMA_CYCLIC, priv->slave.cap_mask); 1239 dma_cap_set(DMA_SLAVE, priv->slave.cap_mask); 1240 1241 INIT_LIST_HEAD(&priv->slave.channels); 1242 priv->slave.device_free_chan_resources = sun4i_dma_free_chan_resources; 1243 priv->slave.device_tx_status = sun4i_dma_tx_status; 1244 priv->slave.device_issue_pending = sun4i_dma_issue_pending; 1245 priv->slave.device_prep_slave_sg = sun4i_dma_prep_slave_sg; 1246 priv->slave.device_prep_dma_memcpy = sun4i_dma_prep_dma_memcpy; 1247 priv->slave.device_prep_dma_cyclic = sun4i_dma_prep_dma_cyclic; 1248 priv->slave.device_config = sun4i_dma_config; 1249 priv->slave.device_terminate_all = sun4i_dma_terminate_all; 1250 priv->slave.copy_align = 2; 1251 priv->slave.src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) | 1252 BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) | 1253 BIT(DMA_SLAVE_BUSWIDTH_4_BYTES); 1254 priv->slave.dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) | 1255 BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) | 1256 BIT(DMA_SLAVE_BUSWIDTH_4_BYTES); 1257 priv->slave.directions = BIT(DMA_DEV_TO_MEM) | 1258 BIT(DMA_MEM_TO_DEV); 1259 priv->slave.residue_granularity = DMA_RESIDUE_GRANULARITY_BURST; 1260 1261 priv->slave.dev = &pdev->dev; 1262 1263 priv->pchans = devm_kcalloc(&pdev->dev, priv->cfg->dma_nr_max_channels, 1264 sizeof(struct sun4i_dma_pchan), GFP_KERNEL); 1265 priv->vchans = devm_kcalloc(&pdev->dev, SUN4I_DMA_NR_MAX_VCHANS, 1266 sizeof(struct sun4i_dma_vchan), GFP_KERNEL); 1267 priv->pchans_used = devm_kcalloc(&pdev->dev, 1268 BITS_TO_LONGS(priv->cfg->dma_nr_max_channels), 1269 sizeof(unsigned long), GFP_KERNEL); 1270 if (!priv->vchans || !priv->pchans || !priv->pchans_used) 1271 return -ENOMEM; 1272 1273 /* 1274 * [0..priv->cfg->ndma_nr_max_channels) are normal pchans, and 1275 * [priv->cfg->ndma_nr_max_channels..priv->cfg->dma_nr_max_channels) are 1276 * dedicated ones 1277 */ 1278 for (i = 0; i < priv->cfg->ndma_nr_max_channels; i++) 1279 priv->pchans[i].base = priv->base + 1280 SUN4I_NDMA_CHANNEL_REG_BASE(i); 1281 1282 for (j = 0; i < priv->cfg->dma_nr_max_channels; i++, j++) { 1283 priv->pchans[i].base = priv->base + 1284 SUN4I_DDMA_CHANNEL_REG_BASE(j); 1285 priv->pchans[i].is_dedicated = 1; 1286 } 1287 1288 for (i = 0; i < SUN4I_DMA_NR_MAX_VCHANS; i++) { 1289 struct sun4i_dma_vchan *vchan = &priv->vchans[i]; 1290 1291 spin_lock_init(&vchan->vc.lock); 1292 vchan->vc.desc_free = sun4i_dma_free_contract; 1293 vchan_init(&vchan->vc, &priv->slave); 1294 } 1295 1296 ret = clk_prepare_enable(priv->clk); 1297 if (ret) { 1298 dev_err(&pdev->dev, "Couldn't enable the clock\n"); 1299 return ret; 1300 } 1301 1302 /* Deassert the reset control */ 1303 ret = reset_control_deassert(priv->rst); 1304 if (ret) { 1305 dev_err(&pdev->dev, 1306 "Failed to deassert the reset control\n"); 1307 goto err_clk_disable; 1308 } 1309 1310 /* 1311 * Make sure the IRQs are all disabled and accounted for. The bootloader 1312 * likes to leave these dirty 1313 */ 1314 writel(0, priv->base + SUN4I_DMA_IRQ_ENABLE_REG); 1315 writel(0xFFFFFFFF, priv->base + SUN4I_DMA_IRQ_PENDING_STATUS_REG); 1316 1317 ret = devm_request_irq(&pdev->dev, priv->irq, sun4i_dma_interrupt, 1318 0, dev_name(&pdev->dev), priv); 1319 if (ret) { 1320 dev_err(&pdev->dev, "Cannot request IRQ\n"); 1321 goto err_clk_disable; 1322 } 1323 1324 ret = dma_async_device_register(&priv->slave); 1325 if (ret) { 1326 dev_warn(&pdev->dev, "Failed to register DMA engine device\n"); 1327 goto err_clk_disable; 1328 } 1329 1330 ret = of_dma_controller_register(pdev->dev.of_node, sun4i_dma_of_xlate, 1331 priv); 1332 if (ret) { 1333 dev_err(&pdev->dev, "of_dma_controller_register failed\n"); 1334 goto err_dma_unregister; 1335 } 1336 1337 dev_dbg(&pdev->dev, "Successfully probed SUN4I_DMA\n"); 1338 1339 return 0; 1340 1341 err_dma_unregister: 1342 dma_async_device_unregister(&priv->slave); 1343 err_clk_disable: 1344 clk_disable_unprepare(priv->clk); 1345 return ret; 1346 } 1347
On Sun, Oct 27, 2024 at 10:14:32AM +0100, Csókás, Bence wrote: > From: Mesih Kilinc <mesihkilinc@gmail.com> > > Allwinner suniv F1C100s has a reset bit for DMA in CCU. Sun4i do not > has this bit but in order to support suniv we need to add it. So add > support for reset bit. > > static struct sun4i_dma_dev *to_sun4i_dma_dev(struct dma_device *dev) > @@ -1215,6 +1218,15 @@ static int sun4i_dma_probe(struct platform_device *pdev) > return PTR_ERR(priv->clk); > } > > + if (priv->cfg->has_reset) { > + priv->rst = devm_reset_control_get_exclusive(&pdev->dev, > + NULL); > + if (IS_ERR(priv->rst)) { > + dev_err_probe(&pdev->dev, "Failed to get reset control\n"); syntax is: return dev_err_probe() Best regards, Krzysztof
Hi, On 2024. 10. 27. 21:42, Krzysztof Kozlowski wrote: > On Sun, Oct 27, 2024 at 10:14:32AM +0100, Csókás, Bence wrote: >> From: Mesih Kilinc <mesihkilinc@gmail.com> >> >> Allwinner suniv F1C100s has a reset bit for DMA in CCU. Sun4i do not >> has this bit but in order to support suniv we need to add it. So add >> support for reset bit. >> >> static struct sun4i_dma_dev *to_sun4i_dma_dev(struct dma_device *dev) >> @@ -1215,6 +1218,15 @@ static int sun4i_dma_probe(struct platform_device *pdev) >> return PTR_ERR(priv->clk); >> } >> >> + if (priv->cfg->has_reset) { >> + priv->rst = devm_reset_control_get_exclusive(&pdev->dev, >> + NULL); >> + if (IS_ERR(priv->rst)) { >> + dev_err_probe(&pdev->dev, "Failed to get reset control\n"); > > syntax is: return dev_err_probe() > > Best regards, > Krzysztof Thanks! And regarding v3 of this patch, I have `clk_disable_unprepare()` after `dev_err_probe()`, I assume I have to let that be i.e. not return immediately?
On Mon, Oct 28, 2024 at 3:37 PM Csókás Bence <csokas.bence@prolan.hu> wrote: > > Hi, > > On 2024. 10. 27. 21:42, Krzysztof Kozlowski wrote: > > On Sun, Oct 27, 2024 at 10:14:32AM +0100, Csókás, Bence wrote: > >> From: Mesih Kilinc <mesihkilinc@gmail.com> > >> > >> Allwinner suniv F1C100s has a reset bit for DMA in CCU. Sun4i do not > >> has this bit but in order to support suniv we need to add it. So add > >> support for reset bit. > >> > >> static struct sun4i_dma_dev *to_sun4i_dma_dev(struct dma_device *dev) > >> @@ -1215,6 +1218,15 @@ static int sun4i_dma_probe(struct platform_device *pdev) > >> return PTR_ERR(priv->clk); > >> } > >> > >> + if (priv->cfg->has_reset) { > >> + priv->rst = devm_reset_control_get_exclusive(&pdev->dev, > >> + NULL); > >> + if (IS_ERR(priv->rst)) { > >> + dev_err_probe(&pdev->dev, "Failed to get reset control\n"); > > > > syntax is: return dev_err_probe() > > > > Best regards, > > Krzysztof > > Thanks! And regarding v3 of this patch, I have `clk_disable_unprepare()` > after `dev_err_probe()`, I assume I have to let that be i.e. not return > immediately? I suggest adding a patch to switch the clk API calls to devm_clk_get_enabled() which handles all the cleanup. Similarly you can switch to devm_reset_control_get_exclusive_deasserted() for this patch. ChenYu
On 28/10/2024 08:37, Csókás Bence wrote: > Hi, > > On 2024. 10. 27. 21:42, Krzysztof Kozlowski wrote: >> On Sun, Oct 27, 2024 at 10:14:32AM +0100, Csókás, Bence wrote: >>> From: Mesih Kilinc <mesihkilinc@gmail.com> >>> >>> Allwinner suniv F1C100s has a reset bit for DMA in CCU. Sun4i do not >>> has this bit but in order to support suniv we need to add it. So add >>> support for reset bit. >>> >>> static struct sun4i_dma_dev *to_sun4i_dma_dev(struct dma_device *dev) >>> @@ -1215,6 +1218,15 @@ static int sun4i_dma_probe(struct platform_device *pdev) >>> return PTR_ERR(priv->clk); >>> } >>> >>> + if (priv->cfg->has_reset) { >>> + priv->rst = devm_reset_control_get_exclusive(&pdev->dev, >>> + NULL); >>> + if (IS_ERR(priv->rst)) { >>> + dev_err_probe(&pdev->dev, "Failed to get reset control\n"); >> >> syntax is: return dev_err_probe() >> >> Best regards, >> Krzysztof > > Thanks! And regarding v3 of this patch, I have `clk_disable_unprepare()` No, you do not. Read your code correctly. + dev_err_probe(&pdev->dev, "Failed to get reset control\n"); + return PTR_ERR(priv->rst); Where is here clk_disable_unprepare()? Best regards, Krzysztof
On 2024. 10. 28. 8:44, Chen-Yu Tsai wrote: > I suggest adding a patch to switch the clk API calls to devm_clk_get_enabled() > which handles all the cleanup. Similarly you can switch to > > devm_reset_control_get_exclusive_deasserted() > > for this patch. > > > ChenYu Huh, that's a new API! Thanks, I'll switch to that then. Regarding the change to devm_clk_get_enabled(), I think that should be a separate patch from this series, where all the pre-existing dev_err()'s get changed as well. If someone wants to work on that, go ahead, but if no one does then after this series is merged I might get around to that too. Bence
On 2024. 10. 28. 14:12, Csókás Bence wrote: > > > On 2024. 10. 28. 8:44, Chen-Yu Tsai wrote: >> I suggest adding a patch to switch the clk API calls to >> devm_clk_get_enabled() >> which handles all the cleanup. Similarly you can switch to >> >> devm_reset_control_get_exclusive_deasserted() >> >> for this patch. >> >> >> ChenYu > > Huh, that's a new API! Thanks, I'll switch to that then. Actually, it doesn't seem to be merged to Linus' tree yet. I'll probably just switch it after it is merged. Bence
diff --git a/drivers/dma/sun4i-dma.c b/drivers/dma/sun4i-dma.c index d472f57a39ea..0a45089461e1 100644 --- a/drivers/dma/sun4i-dma.c +++ b/drivers/dma/sun4i-dma.c @@ -15,6 +15,7 @@ #include <linux/of_dma.h> #include <linux/of_device.h> #include <linux/platform_device.h> +#include <linux/reset.h> #include <linux/slab.h> #include <linux/spinlock.h> @@ -159,6 +160,7 @@ struct sun4i_dma_config { u8 ddma_drq_sdram; u8 max_burst; + bool has_reset; }; struct sun4i_dma_pchan { @@ -208,6 +210,7 @@ struct sun4i_dma_dev { int irq; spinlock_t lock; const struct sun4i_dma_config *cfg; + struct reset_control *rst; }; static struct sun4i_dma_dev *to_sun4i_dma_dev(struct dma_device *dev) @@ -1215,6 +1218,15 @@ static int sun4i_dma_probe(struct platform_device *pdev) return PTR_ERR(priv->clk); } + if (priv->cfg->has_reset) { + priv->rst = devm_reset_control_get_exclusive(&pdev->dev, + NULL); + if (IS_ERR(priv->rst)) { + dev_err_probe(&pdev->dev, "Failed to get reset control\n"); + return PTR_ERR(priv->rst); + } + } + platform_set_drvdata(pdev, priv); spin_lock_init(&priv->lock); @@ -1287,6 +1299,14 @@ static int sun4i_dma_probe(struct platform_device *pdev) return ret; } + /* Deassert the reset control */ + ret = reset_control_deassert(priv->rst); + if (ret) { + dev_err(&pdev->dev, + "Failed to deassert the reset control\n"); + goto err_clk_disable; + } + /* * Make sure the IRQs are all disabled and accounted for. The bootloader * likes to leave these dirty @@ -1356,6 +1376,7 @@ static struct sun4i_dma_config sun4i_a10_dma_cfg = { .ddma_drq_sdram = SUN4I_DDMA_DRQ_TYPE_SDRAM, .max_burst = SUN4I_MAX_BURST, + .has_reset = false, }; static const struct of_device_id sun4i_dma_match[] = {