Skip to content

Commit d709697

Browse files
amboarcminyard
amboar
authored and
cminyard
committed
ipmi: kcs_bmc: Turn the driver data-structures inside-out
Make the KCS device drivers responsible for allocating their own memory. Until now the private data for the device driver was allocated internal to the private data for the chardev interface. This coupling required the slightly awkward API of passing through the struct size for the driver private data to the chardev constructor, and then retrieving a pointer to the driver private data from the allocated chardev memory. In addition to being awkward, the arrangement prevents the implementation of alternative userspace interfaces as the device driver private data is not independent. Peel a layer off the onion and turn the data-structures inside out by exploiting container_of() and embedding `struct kcs_device` in the driver private data. Signed-off-by: Andrew Jeffery <andrew@aj.id.au> Reviewed-by: Zev Weiss <zweiss@equinix.com> Message-Id: <20210608104757.582199-6-andrew@aj.id.au> Signed-off-by: Corey Minyard <cminyard@mvista.com>
1 parent 55ab48b commit d709697

5 files changed

Lines changed: 110 additions & 67 deletions

File tree

‎drivers/char/ipmi/kcs_bmc.c‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,21 @@ int kcs_bmc_handle_event(struct kcs_bmc *kcs_bmc)
4444
}
4545
EXPORT_SYMBOL(kcs_bmc_handle_event);
4646

47-
struct kcs_bmc *kcs_bmc_ipmi_alloc(struct device *dev, int sizeof_priv, u32 channel);
48-
struct kcs_bmc *kcs_bmc_alloc(struct device *dev, int sizeof_priv, u32 channel)
47+
int kcs_bmc_ipmi_add_device(struct kcs_bmc *kcs_bmc);
48+
int kcs_bmc_add_device(struct kcs_bmc *kcs_bmc)
4949
{
50-
return kcs_bmc_ipmi_alloc(dev, sizeof_priv, channel);
50+
return kcs_bmc_ipmi_add_device(kcs_bmc);
5151
}
52-
EXPORT_SYMBOL(kcs_bmc_alloc);
52+
EXPORT_SYMBOL(kcs_bmc_add_device);
53+
54+
int kcs_bmc_ipmi_remove_device(struct kcs_bmc *kcs_bmc);
55+
void kcs_bmc_remove_device(struct kcs_bmc *kcs_bmc)
56+
{
57+
if (kcs_bmc_ipmi_remove_device(kcs_bmc))
58+
pr_warn("Failed to remove device for KCS channel %d\n",
59+
kcs_bmc->channel);
60+
}
61+
EXPORT_SYMBOL(kcs_bmc_remove_device);
5362

5463
MODULE_LICENSE("GPL v2");
5564
MODULE_AUTHOR("Haiyue Wang <haiyue.wang@linux.intel.com>");

‎drivers/char/ipmi/kcs_bmc.h‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@ struct kcs_ioreg {
6767
};
6868

6969
struct kcs_bmc {
70+
struct device *dev;
71+
7072
spinlock_t lock;
7173

7274
u32 channel;
@@ -94,17 +96,11 @@ struct kcs_bmc {
9496
u8 *kbuffer;
9597

9698
struct miscdevice miscdev;
97-
98-
unsigned long priv[];
9999
};
100100

101-
static inline void *kcs_bmc_priv(struct kcs_bmc *kcs_bmc)
102-
{
103-
return kcs_bmc->priv;
104-
}
105-
106101
int kcs_bmc_handle_event(struct kcs_bmc *kcs_bmc);
107-
struct kcs_bmc *kcs_bmc_alloc(struct device *dev, int sizeof_priv, u32 channel);
102+
int kcs_bmc_add_device(struct kcs_bmc *kcs_bmc);
103+
void kcs_bmc_remove_device(struct kcs_bmc *kcs_bmc);
108104

109105
u8 kcs_bmc_read_data(struct kcs_bmc *kcs_bmc);
110106
void kcs_bmc_write_data(struct kcs_bmc *kcs_bmc, u8 data);

‎drivers/char/ipmi/kcs_bmc_aspeed.c‎

Lines changed: 30 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@
6161
#define LPC_STR4 0x11C
6262

6363
struct aspeed_kcs_bmc {
64+
struct kcs_bmc kcs_bmc;
65+
6466
struct regmap *map;
6567
};
6668

@@ -69,9 +71,14 @@ struct aspeed_kcs_of_ops {
6971
int (*get_io_address)(struct platform_device *pdev);
7072
};
7173

74+
static inline struct aspeed_kcs_bmc *to_aspeed_kcs_bmc(struct kcs_bmc *kcs_bmc)
75+
{
76+
return container_of(kcs_bmc, struct aspeed_kcs_bmc, kcs_bmc);
77+
}
78+
7279
static u8 aspeed_kcs_inb(struct kcs_bmc *kcs_bmc, u32 reg)
7380
{
74-
struct aspeed_kcs_bmc *priv = kcs_bmc_priv(kcs_bmc);
81+
struct aspeed_kcs_bmc *priv = to_aspeed_kcs_bmc(kcs_bmc);
7582
u32 val = 0;
7683
int rc;
7784

@@ -83,7 +90,7 @@ static u8 aspeed_kcs_inb(struct kcs_bmc *kcs_bmc, u32 reg)
8390

8491
static void aspeed_kcs_outb(struct kcs_bmc *kcs_bmc, u32 reg, u8 data)
8592
{
86-
struct aspeed_kcs_bmc *priv = kcs_bmc_priv(kcs_bmc);
93+
struct aspeed_kcs_bmc *priv = to_aspeed_kcs_bmc(kcs_bmc);
8794
int rc;
8895

8996
rc = regmap_write(priv->map, reg, data);
@@ -92,7 +99,7 @@ static void aspeed_kcs_outb(struct kcs_bmc *kcs_bmc, u32 reg, u8 data)
9299

93100
static void aspeed_kcs_updateb(struct kcs_bmc *kcs_bmc, u32 reg, u8 mask, u8 val)
94101
{
95-
struct aspeed_kcs_bmc *priv = kcs_bmc_priv(kcs_bmc);
102+
struct aspeed_kcs_bmc *priv = to_aspeed_kcs_bmc(kcs_bmc);
96103
int rc;
97104

98105
rc = regmap_update_bits(priv->map, reg, mask, val);
@@ -114,7 +121,7 @@ static void aspeed_kcs_updateb(struct kcs_bmc *kcs_bmc, u32 reg, u8 mask, u8 val
114121
*/
115122
static void aspeed_kcs_set_address(struct kcs_bmc *kcs_bmc, u16 addr)
116123
{
117-
struct aspeed_kcs_bmc *priv = kcs_bmc_priv(kcs_bmc);
124+
struct aspeed_kcs_bmc *priv = to_aspeed_kcs_bmc(kcs_bmc);
118125

119126
switch (kcs_bmc->channel) {
120127
case 1:
@@ -148,7 +155,7 @@ static void aspeed_kcs_set_address(struct kcs_bmc *kcs_bmc, u16 addr)
148155

149156
static void aspeed_kcs_enable_channel(struct kcs_bmc *kcs_bmc, bool enable)
150157
{
151-
struct aspeed_kcs_bmc *priv = kcs_bmc_priv(kcs_bmc);
158+
struct aspeed_kcs_bmc *priv = to_aspeed_kcs_bmc(kcs_bmc);
152159

153160
switch (kcs_bmc->channel) {
154161
case 1:
@@ -325,17 +332,16 @@ static int aspeed_kcs_of_v2_get_io_address(struct platform_device *pdev)
325332
static int aspeed_kcs_probe(struct platform_device *pdev)
326333
{
327334
const struct aspeed_kcs_of_ops *ops;
328-
struct device *dev = &pdev->dev;
329335
struct aspeed_kcs_bmc *priv;
330336
struct kcs_bmc *kcs_bmc;
331337
struct device_node *np;
332338
int rc, channel, addr;
333339

334-
np = dev->of_node->parent;
340+
np = pdev->dev.of_node->parent;
335341
if (!of_device_is_compatible(np, "aspeed,ast2400-lpc-v2") &&
336342
!of_device_is_compatible(np, "aspeed,ast2500-lpc-v2") &&
337343
!of_device_is_compatible(np, "aspeed,ast2600-lpc-v2")) {
338-
dev_err(dev, "unsupported LPC device binding\n");
344+
dev_err(&pdev->dev, "unsupported LPC device binding\n");
339345
return -ENODEV;
340346
}
341347
ops = of_device_get_match_data(&pdev->dev);
@@ -346,20 +352,22 @@ static int aspeed_kcs_probe(struct platform_device *pdev)
346352
if (channel < 0)
347353
return channel;
348354

349-
kcs_bmc = kcs_bmc_alloc(&pdev->dev, sizeof(struct aspeed_kcs_bmc), channel);
350-
if (!kcs_bmc)
355+
addr = ops->get_io_address(pdev);
356+
if (addr < 0)
357+
return addr;
358+
359+
priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
360+
if (!priv)
351361
return -ENOMEM;
352362

363+
kcs_bmc = &priv->kcs_bmc;
364+
kcs_bmc->dev = &pdev->dev;
365+
kcs_bmc->channel = channel;
353366
kcs_bmc->ioreg = ast_kcs_bmc_ioregs[channel - 1];
354367
kcs_bmc->io_inputb = aspeed_kcs_inb;
355368
kcs_bmc->io_outputb = aspeed_kcs_outb;
356369
kcs_bmc->io_updateb = aspeed_kcs_updateb;
357370

358-
addr = ops->get_io_address(pdev);
359-
if (addr < 0)
360-
return addr;
361-
362-
priv = kcs_bmc_priv(kcs_bmc);
363371
priv->map = syscon_node_to_regmap(pdev->dev.parent->of_node);
364372
if (IS_ERR(priv->map)) {
365373
dev_err(&pdev->dev, "Couldn't get regmap\n");
@@ -372,29 +380,27 @@ static int aspeed_kcs_probe(struct platform_device *pdev)
372380
if (rc)
373381
return rc;
374382

375-
dev_set_drvdata(dev, kcs_bmc);
383+
platform_set_drvdata(pdev, priv);
376384

377385
aspeed_kcs_enable_channel(kcs_bmc, true);
378386

379-
rc = misc_register(&kcs_bmc->miscdev);
387+
rc = kcs_bmc_add_device(&priv->kcs_bmc);
380388
if (rc) {
381-
dev_err(dev, "Unable to register device\n");
389+
dev_warn(&pdev->dev, "Failed to register channel %d: %d\n", kcs_bmc->channel, rc);
382390
return rc;
383391
}
384392

385-
dev_dbg(&pdev->dev,
386-
"Probed KCS device %d (IDR=0x%x, ODR=0x%x, STR=0x%x)\n",
387-
kcs_bmc->channel, kcs_bmc->ioreg.idr, kcs_bmc->ioreg.odr,
388-
kcs_bmc->ioreg.str);
393+
dev_info(&pdev->dev, "Initialised channel %d at 0x%x\n", kcs_bmc->channel, addr);
389394

390395
return 0;
391396
}
392397

393398
static int aspeed_kcs_remove(struct platform_device *pdev)
394399
{
395-
struct kcs_bmc *kcs_bmc = dev_get_drvdata(&pdev->dev);
400+
struct aspeed_kcs_bmc *priv = platform_get_drvdata(pdev);
401+
struct kcs_bmc *kcs_bmc = &priv->kcs_bmc;
396402

397-
misc_deregister(&kcs_bmc->miscdev);
403+
kcs_bmc_remove_device(kcs_bmc);
398404

399405
return 0;
400406
}

‎drivers/char/ipmi/kcs_bmc_cdev_ipmi.c‎

Lines changed: 41 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,7 @@ static int kcs_bmc_ipmi_release(struct inode *inode, struct file *filp)
382382
return 0;
383383
}
384384

385-
static const struct file_operations kcs_bmc_fops = {
385+
static const struct file_operations kcs_bmc_ipmi_fops = {
386386
.owner = THIS_MODULE,
387387
.open = kcs_bmc_ipmi_open,
388388
.read = kcs_bmc_ipmi_read,
@@ -392,36 +392,58 @@ static const struct file_operations kcs_bmc_fops = {
392392
.unlocked_ioctl = kcs_bmc_ipmi_ioctl,
393393
};
394394

395-
struct kcs_bmc *kcs_bmc_ipmi_alloc(struct device *dev, int sizeof_priv, u32 channel);
396-
struct kcs_bmc *kcs_bmc_ipmi_alloc(struct device *dev, int sizeof_priv, u32 channel)
395+
int kcs_bmc_ipmi_add_device(struct kcs_bmc *kcs_bmc);
396+
int kcs_bmc_ipmi_add_device(struct kcs_bmc *kcs_bmc)
397397
{
398-
struct kcs_bmc *kcs_bmc;
399-
400-
kcs_bmc = devm_kzalloc(dev, sizeof(*kcs_bmc) + sizeof_priv, GFP_KERNEL);
401-
if (!kcs_bmc)
402-
return NULL;
398+
int rc;
403399

404400
spin_lock_init(&kcs_bmc->lock);
405-
kcs_bmc->channel = channel;
406-
407401
mutex_init(&kcs_bmc->mutex);
408402
init_waitqueue_head(&kcs_bmc->queue);
409403

410-
kcs_bmc->data_in = devm_kmalloc(dev, KCS_MSG_BUFSIZ, GFP_KERNEL);
411-
kcs_bmc->data_out = devm_kmalloc(dev, KCS_MSG_BUFSIZ, GFP_KERNEL);
412-
kcs_bmc->kbuffer = devm_kmalloc(dev, KCS_MSG_BUFSIZ, GFP_KERNEL);
404+
kcs_bmc->data_in = devm_kmalloc(kcs_bmc->dev, KCS_MSG_BUFSIZ, GFP_KERNEL);
405+
kcs_bmc->data_out = devm_kmalloc(kcs_bmc->dev, KCS_MSG_BUFSIZ, GFP_KERNEL);
406+
kcs_bmc->kbuffer = devm_kmalloc(kcs_bmc->dev, KCS_MSG_BUFSIZ, GFP_KERNEL);
413407

414408
kcs_bmc->miscdev.minor = MISC_DYNAMIC_MINOR;
415-
kcs_bmc->miscdev.name = devm_kasprintf(dev, GFP_KERNEL, "%s%u",
416-
DEVICE_NAME, channel);
409+
kcs_bmc->miscdev.name = devm_kasprintf(kcs_bmc->dev, GFP_KERNEL, "%s%u",
410+
DEVICE_NAME, kcs_bmc->channel);
417411
if (!kcs_bmc->data_in || !kcs_bmc->data_out || !kcs_bmc->kbuffer ||
418412
!kcs_bmc->miscdev.name)
419-
return NULL;
420-
kcs_bmc->miscdev.fops = &kcs_bmc_fops;
413+
return -ENOMEM;
414+
415+
kcs_bmc->miscdev.fops = &kcs_bmc_ipmi_fops;
416+
417+
rc = misc_register(&kcs_bmc->miscdev);
418+
if (rc) {
419+
dev_err(kcs_bmc->dev, "Unable to register device: %d\n", rc);
420+
return rc;
421+
}
422+
423+
dev_info(kcs_bmc->dev, "Initialised IPMI client for channel %d", kcs_bmc->channel);
424+
425+
return 0;
426+
}
427+
EXPORT_SYMBOL(kcs_bmc_ipmi_add_device);
421428

422-
return kcs_bmc;
429+
int kcs_bmc_ipmi_remove_device(struct kcs_bmc *kcs_bmc);
430+
int kcs_bmc_ipmi_remove_device(struct kcs_bmc *kcs_bmc)
431+
{
432+
misc_deregister(&kcs_bmc->miscdev);
433+
434+
spin_lock_irq(&kcs_bmc->lock);
435+
kcs_bmc->running = 0;
436+
kcs_bmc_ipmi_force_abort(kcs_bmc);
437+
spin_unlock_irq(&kcs_bmc->lock);
438+
439+
devm_kfree(kcs_bmc->dev, kcs_bmc->kbuffer);
440+
devm_kfree(kcs_bmc->dev, kcs_bmc->data_out);
441+
devm_kfree(kcs_bmc->dev, kcs_bmc->data_in);
442+
devm_kfree(kcs_bmc->dev, kcs_bmc);
443+
444+
return 0;
423445
}
424-
EXPORT_SYMBOL(kcs_bmc_ipmi_alloc);
446+
EXPORT_SYMBOL(kcs_bmc_ipmi_remove_device);
425447

426448
MODULE_LICENSE("GPL v2");
427449
MODULE_AUTHOR("Haiyue Wang <haiyue.wang@linux.intel.com>");

‎drivers/char/ipmi/kcs_bmc_npcm7xx.c‎

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,8 @@ struct npcm7xx_kcs_reg {
6565
};
6666

6767
struct npcm7xx_kcs_bmc {
68+
struct kcs_bmc kcs_bmc;
69+
6870
struct regmap *map;
6971

7072
const struct npcm7xx_kcs_reg *reg;
@@ -76,9 +78,14 @@ static const struct npcm7xx_kcs_reg npcm7xx_kcs_reg_tbl[KCS_CHANNEL_MAX] = {
7678
{ .sts = KCS3ST, .dob = KCS3DO, .dib = KCS3DI, .ctl = KCS3CTL, .ie = KCS3IE },
7779
};
7880

81+
static inline struct npcm7xx_kcs_bmc *to_npcm7xx_kcs_bmc(struct kcs_bmc *kcs_bmc)
82+
{
83+
return container_of(kcs_bmc, struct npcm7xx_kcs_bmc, kcs_bmc);
84+
}
85+
7986
static u8 npcm7xx_kcs_inb(struct kcs_bmc *kcs_bmc, u32 reg)
8087
{
81-
struct npcm7xx_kcs_bmc *priv = kcs_bmc_priv(kcs_bmc);
88+
struct npcm7xx_kcs_bmc *priv = to_npcm7xx_kcs_bmc(kcs_bmc);
8289
u32 val = 0;
8390
int rc;
8491

@@ -90,7 +97,7 @@ static u8 npcm7xx_kcs_inb(struct kcs_bmc *kcs_bmc, u32 reg)
9097

9198
static void npcm7xx_kcs_outb(struct kcs_bmc *kcs_bmc, u32 reg, u8 data)
9299
{
93-
struct npcm7xx_kcs_bmc *priv = kcs_bmc_priv(kcs_bmc);
100+
struct npcm7xx_kcs_bmc *priv = to_npcm7xx_kcs_bmc(kcs_bmc);
94101
int rc;
95102

96103
rc = regmap_write(priv->map, reg, data);
@@ -99,7 +106,7 @@ static void npcm7xx_kcs_outb(struct kcs_bmc *kcs_bmc, u32 reg, u8 data)
99106

100107
static void npcm7xx_kcs_updateb(struct kcs_bmc *kcs_bmc, u32 reg, u8 mask, u8 data)
101108
{
102-
struct npcm7xx_kcs_bmc *priv = kcs_bmc_priv(kcs_bmc);
109+
struct npcm7xx_kcs_bmc *priv = to_npcm7xx_kcs_bmc(kcs_bmc);
103110
int rc;
104111

105112
rc = regmap_update_bits(priv->map, reg, mask, data);
@@ -108,7 +115,7 @@ static void npcm7xx_kcs_updateb(struct kcs_bmc *kcs_bmc, u32 reg, u8 mask, u8 da
108115

109116
static void npcm7xx_kcs_enable_channel(struct kcs_bmc *kcs_bmc, bool enable)
110117
{
111-
struct npcm7xx_kcs_bmc *priv = kcs_bmc_priv(kcs_bmc);
118+
struct npcm7xx_kcs_bmc *priv = to_npcm7xx_kcs_bmc(kcs_bmc);
112119

113120
regmap_update_bits(priv->map, priv->reg->ctl, KCS_CTL_IBFIE,
114121
enable ? KCS_CTL_IBFIE : 0);
@@ -155,35 +162,37 @@ static int npcm7xx_kcs_probe(struct platform_device *pdev)
155162
return -ENODEV;
156163
}
157164

158-
kcs_bmc = kcs_bmc_alloc(dev, sizeof(*priv), chan);
159-
if (!kcs_bmc)
165+
priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
166+
if (!priv)
160167
return -ENOMEM;
161168

162-
priv = kcs_bmc_priv(kcs_bmc);
163169
priv->map = syscon_node_to_regmap(dev->parent->of_node);
164170
if (IS_ERR(priv->map)) {
165171
dev_err(dev, "Couldn't get regmap\n");
166172
return -ENODEV;
167173
}
168174
priv->reg = &npcm7xx_kcs_reg_tbl[chan - 1];
169175

176+
kcs_bmc = &priv->kcs_bmc;
177+
kcs_bmc->dev = &pdev->dev;
178+
kcs_bmc->channel = chan;
170179
kcs_bmc->ioreg.idr = priv->reg->dib;
171180
kcs_bmc->ioreg.odr = priv->reg->dob;
172181
kcs_bmc->ioreg.str = priv->reg->sts;
173182
kcs_bmc->io_inputb = npcm7xx_kcs_inb;
174183
kcs_bmc->io_outputb = npcm7xx_kcs_outb;
175184
kcs_bmc->io_updateb = npcm7xx_kcs_updateb;
176185

177-
dev_set_drvdata(dev, kcs_bmc);
186+
platform_set_drvdata(pdev, priv);
178187

179188
npcm7xx_kcs_enable_channel(kcs_bmc, true);
180189
rc = npcm7xx_kcs_config_irq(kcs_bmc, pdev);
181190
if (rc)
182191
return rc;
183192

184-
rc = misc_register(&kcs_bmc->miscdev);
193+
rc = kcs_bmc_add_device(kcs_bmc);
185194
if (rc) {
186-
dev_err(dev, "Unable to register device\n");
195+
dev_warn(&pdev->dev, "Failed to register channel %d: %d\n", kcs_bmc->channel, rc);
187196
return rc;
188197
}
189198

@@ -196,9 +205,10 @@ static int npcm7xx_kcs_probe(struct platform_device *pdev)
196205

197206
static int npcm7xx_kcs_remove(struct platform_device *pdev)
198207
{
199-
struct kcs_bmc *kcs_bmc = dev_get_drvdata(&pdev->dev);
208+
struct npcm7xx_kcs_bmc *priv = platform_get_drvdata(pdev);
209+
struct kcs_bmc *kcs_bmc = &priv->kcs_bmc;
200210

201-
misc_deregister(&kcs_bmc->miscdev);
211+
kcs_bmc_remove_device(kcs_bmc);
202212

203213
return 0;
204214
}

0 commit comments

Comments
 (0)